## Context
the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens.
I have this _Verifier_ support basically implemented, but this PR is not it.
This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code.
I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task.
## The problem being addressed
Configuration of JWT signing secrets/keys was inconsistent:
Under `[oauth2]` the full configuration set was supported:
- `JWT_SIGNING_ALGORITHM` configured the algorithm
- `JWT_SECRET` configured a literal secret for symmetric algorithms
- `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms
- `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms
For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256`
For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`.
## ini unification
The proposed code centralizes ini parsing to always support the following ini keys:
- `[pfx]SIGNING_ALGORITHM` determines the algorithm
- `[pfx]SECRET` is a literal secret for symmetric algorithms
- `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms
- `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms
`[pfx]` is specific to the module and chosen to support the existing ini keys
Centralizing this code and unifying the ini keys will come handy for at least the following reasons:
- consistent behavior across modules is easier to understand
- less duplicated code
- easier to expand later, which is my main motivation
## implementation notes
as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up.
To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where
- the settings component creates missing symmetric signing keys and writes them to the .ini
- the settings component creates a simple configuration struct
- which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist.
I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases.
## Commits
Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference:
### Refactor signing key initalization and oauth2 use of it
This commit is the first in a series towards the goal of addressing the
FIXME comment in modules/setting/oauth2.go to remove
GeneralTokenSigningSecret
To do it properly, the task also requires addition of signing secret/key
rotation: We ultimately want to be able to change a signing key, but
continue to accept the previous one. This is particularly relevant to
offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new,
specific component key configuration, where it should be possible to add
the former JWT_SECRET as a key accepted for verification to enable a
seamless transition.
This perspective, in turn, calls for refactoring of the existing secret
initialization code to centralize the common functions of parsing
signing key related configuration directives: The oauth2 module
currently is the only component accepting symmetric and asymmetric keys,
with the limitation of the symmetric key being also the
GeneralTokenSigningSecret. Other components either enforce HS256 or
public key algorithms.
We should really give the choice of algorithm selection and avoid code
duplication in other places, so this commit
- generalizes setting parsing into a configuration struct: A prefix can
be provided, with which the common configuration directives are
processed:
- [pfx]SIGNING_ALGORITHM determines the algorithm
- [pfx]SECRET is a literal secret for symmetric algorithms
- [pfx]SECRET_URI is the uri of a secret for symmetric algorithms
- [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms
- which is then accepted by jwtx.InitSigningKey() to create an actual
signing key
The reasons for the two stage process are explained in a long-ish
comment in modules/setting/security.go. In short, other options would
either violate sensible module boundaries or cause too much friction.
These other options have actually been tried, this is take 3 of the
proposed changes.
### Refactor services/lfs: Change token code to use SigningKey
This now also enables use of token algorithms other than HS256.
In this case, signing key initialization also happens during settings
initialization, because LFS is also used in CLI commands.
### Refactor api/actions to use new signingkey API
This now also enables use of symmetric token algorithms.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Nils Goroll <nils.goroll@uplex.de>
Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
Closes: #12204
The underlying git option was already changed in git 2.0.0 to use format `<mode>,<object>,<path>`. See ec160ae12b.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12214
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Robert Wolff <mahlzahn@posteo.de>
Co-committed-by: Robert Wolff <mahlzahn@posteo.de>
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Tests for JavaScript changes
(not applicable — Go-only change)
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
## Summary
Add public REST API endpoints under `/api/v1/` for listing, inspecting, downloading, and deleting Actions artifacts. Previously, artifacts could only be accessed through the web UI or the internal runner API.
### New endpoints
| Method | Path | Description |
|--------|------|-------------|
| `GET` | `/repos/{owner}/{repo}/actions/artifacts` | List all artifacts for a repository |
| `GET` | `/repos/{owner}/{repo}/actions/runs/{run_id}/artifacts` | List artifacts for a workflow run |
| `GET` | `/repos/{owner}/{repo}/actions/artifacts/{artifact_id}` | Get artifact metadata |
| `GET` | `/repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip` | Download artifact as zip |
| `DELETE` | `/repos/{owner}/{repo}/actions/artifacts/{artifact_id}` | Delete an artifact |
- List endpoints support `page`, `limit`, and `name` query parameters
- Both v1-v3 (multi-file, zip on-the-fly) and v4 (single zip) artifact backends are supported
- Expired artifacts are listed with `expired: true` but cannot be downloaded
- Delete requires write permission; all other endpoints require read permission
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12140
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: ShellWen <me@shellwen.com>
Co-committed-by: ShellWen <me@shellwen.com>
Move the logic for handling reruns of Forgejo Action workflows and individual jobs to services. That is a prerequisite for adding the corresponding HTTP API endpoints.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
(can be removed for JavaScript changes)
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Tests for JavaScript changes
(can be removed for Go changes)
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
*The decision if the pull request will be shown in the release notes is up to the mergers / release team.*
The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12141
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Co-committed-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Resources in Forgejo can also be owned by predefined system users like Ghost or Forgejo Actions. Those have negative user IDs, for example, -2 in the case of Forgejo Actions. `OwnerID` checks oftentimes do not take these users into account, because their existence and how they work isn't well known. A [semgrep](https://semgrep.dev/) check is added that flags such suspicious `OwnerID` checks.
See https://codeberg.org/forgejo/forgejo/pulls/12144 for background.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
(can be removed for JavaScript changes)
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Tests for JavaScript changes
(can be removed for Go changes)
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
*The decision if the pull request will be shown in the release notes is up to the mergers / release team.*
The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12184
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Co-committed-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Followup to https://codeberg.org/forgejo/forgejo/pulls/6203
Currently it is logging an error wherever a template is rendered in language that doesn't have all plural strings covered. For example, Esperanto isn't well maintained.
Since more plural strings were migrated in v15 to new format, these errors became much more common. However, for all languages but the base one (English) they are completely harmless and just indicate an incomplete translation.
However, for base (English) they indicate a bug in either template or en-US.json, which should be still logged as an error.
The error is being logged by `LookupPluralByForm`, which is called by `TrPluralStringAllForms` and (`TrPluralString` through `LookupPluralByCount`). I originally intended to just pass log func directly to `LookupPluralByForm` from both, but since `TrPluralString` isn't calling `LookupPluralByForm` directly, it didn't look clean, so I went with passing a flag around instead and implemented logging logic in `LookupPluralByForm` itself.
I little concern is with that the so-called "default lang" is configurable, and if it is configured to something with less than 100% completion, it will cause fallback bugs, as well as a lot of logging of this as an error. But this is why changing "default lang" is a bad idea in the first place, and broken fallbacks should be greater concern than junk in the logs.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12183
Reviewed-by: Beowulf <beowulf@beocode.eu>
Co-authored-by: 0ko <0ko@noreply.codeberg.org>
Co-committed-by: 0ko <0ko@noreply.codeberg.org>
- The documentation has the correct behavior about `linguist-detectable`: In cases where a file should be considered for language statistics, regardless of its category, the linguist-detectable attribute can be used.
- This patch follows that behavior by not skipping the file even if some heuristic would've said to skip the file.
- Document the conditions in more natural language.
- Resolvesforgejo/forgejo#11248
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11685
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
Fixes#11590
When viewing a user's SSH keys, SSH principals are now excluded from the output. This would previously either result in a panic in [OmitEmail](cfd4d53e32/models/asymkey/ssh_key.go (L67)), if the principal name didn't contain any spaces, or truncate the principal name, if it did contain spaces.
The TestExportUserSSHKeys test was also updated and fails if the fix(commit cfcbc33af0) is reverted.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
- [x] `make test`
- [x] `make test-sqlite#TestExportUserSSHKeys`
I have also manually tested the change.
The full integration tests(`make test-sqlite`) report some errors, but I get the same errors without this PR(tested on commit [6a5dda7116](6a5dda7116)).
I have not tested with the other database backends.
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
*The decision if the pull request will be shown in the release notes is up to the mergers / release team.*
The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12079
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Alec Walsh <code@alecwalsh.name>
Co-committed-by: Alec Walsh <code@alecwalsh.name>
I tried a lot, but this seems to work. I know it is ugly, but checking and waiting after every action seems to make it stable. At least it succeeded five times in a row and the CI seemed to be under load due to the dependency updates. Maybe it is worth a try...
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12151
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Allow for filtering users with 2fa enabled as admin. So that it is easy to audit users' settings compliance with iso27001, etc.
Resolves#11800
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12091
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Dominik Zyla <zylad@noreply.codeberg.org>
Co-committed-by: Dominik Zyla <zylad@noreply.codeberg.org>
API calls to `.../api/v1/repos/search?uid=-2&archived=false` currently do not apply the filter `uid` because of the negative value. This can occur when APIs are interacting with `${{ forgejo.token }}` and believe they're operating as the Forgejo Actions user, which has UID -2.
In combination with the security checks that occur in the `/repos/search` API to validate that repositories accessed are visible to the user, this can result in 500 error responses when a more correct expectation would be to receive no repositories:
da8898822c/routers/api/v1/repo/repo.go (L237-L242)
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12144
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
Prevent continued execution of some APIs with error responses that didn't correctly interrupt execution, resulting in bizarre outputs and possibly leaking secure data:
```
> GET /api/v1/repos/search?uid=-2&archived=false HTTP/2
> Host: example.org
> user-agent: curl/7.88.1
> accept: */*
> authorization: bearer ***
>
< HTTP/2 500
< server: nginx
< date: Thu, 16 Apr 2026 14:20:09 GMT
< content-type: application/json;charset=utf-8
< cache-control: max-age=0, private, must-revalidate, no-transform
< x-content-type-options: nosniff
< x-frame-options: SAMEORIGIN
<
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"message":"","url":"https://example.org/api/swagger"}
{"ok":true,"data":[{"id":68,"owner":{"id":1,"login":"mfenniak", ...
```
As these errors only occur on situations that shouldn't be reproducible (minus software bugs), test automation isn't practical.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [ ] `make pr-go` before pushing
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12143
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Cyborus <cyborus@disroot.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This was missed in https://codeberg.org/forgejo/forgejo/pulls/11098.
See https://github.com/go-gitea/gitea/pull/17846 for why this was added in the first place.
Note that this is not backwards compatible. For users with a custom `app.ini`-config this won't work. But it also didn't work with the previous config. This change only aligns it with the default app.ini-path.
Co-authored-by: Jakob Linskeseder <jakob@linskeseder.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11720
Reviewed-by: Beowulf <beowulf@beocode.eu>
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: jaylinski <jaylinski@noreply.codeberg.org>
Co-committed-by: jaylinski <jaylinski@noreply.codeberg.org>
`TestMirrorPull` is currently failing when run on git 2.34.1 in the `testing-integration.yml` workflow: https://codeberg.org/forgejo-integration/forgejo/actions/runs/16661/jobs/1/attempt/1#jobstep-5-2539 Began to fail after #11909 when additional checks on pull mirror configuration was added.
This PR addresses the issue and has been manually tested against the same git version:
```
$ git --version
git version 2.34.1
$ make test-sqlite#TestMirrorPull 2>&1
...
=== TestMirrorPull/migrate_from_repo_config_credentials (tests/integration/mirror_pull_test.go:238)
PASS
```
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12134
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: Beowulf <beowulf@beocode.eu>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
- shrink runner list width (use icons, move details link to runner name)
- add owner to runner details on admin view
- #11516 removed a lot details which makes it much harder for an admin to find a specific runner
---
### admin list
### admin org runner details
### admin repo runner
### individual list
### individual runner details
### tooltips for edit and delete
 
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12113
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Co-committed-by: Michael Kriese <michael.kriese@visualon.de>
While developing tests for #12092, I came across a case where making a comment on a single-commit doesn't include the correct diff for the comment. This is because code comment placement occurs between the PR's base and the commit being viewed, but, that diff could be different from the commit's parent to the commit, which is what is being viewed on a single-commit diff.
Similar to #12055, this PR changes code comments to be more precise in their diff generation by providing the backend with both the base commit (`before_commit_id`) and head commit (`after_commit_id`) currently being viewed. As a result, the diffs attached to comments should exactly match the diffs being viewed by the user when the comment was placed.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12107
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
- It's quite hard to determine when and why this was added here, my best
guess is that this being the "oldest" subcommand at some point loading
the configuration was not unified. Now it is unified in
`prepareWorkPathAndCustomConf` which is run before any subcommand is
run. It determines the work path, custom path and (custom) config and
then loads the settings by calling `LoadCommonSettings`.
- Between `prepareWorkPathAndCustomConf` being called and
`serveInstalled` being called the `setting.CustomConf` is not changed.
There was a possibility this being necessary for install page ->
installed, but the install code already ensures that the new config is
loaded and used.
- Thus calling to load the settings again here is not necessary. There's
a small possibility some settings loading code was written to only work
after being loaded the second time. That's a bug that needs to be fixed,
because all other subcommands does not load the settings twice and would
see a different view of the settings in that case. I don't fear such
code being present here.
- Resolvesforgejo/forgejo#11024
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12111
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
First round of patches to re-enable some lints from my side.
This PR also refactors the general key fetching code quite a bit due to the way it currently worked
with relying on some values being nil sometimes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11253
Reviewed-by: elle <0xllx0@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: famfo <famfo@famfo.xyz>
Co-committed-by: famfo <famfo@famfo.xyz>
With the completion of #12015, when a comment is left on a changed line in a pull request, we track the comment against the line of code with `git blame` and then identify where it currently is in any diff with `git blame --reverse`. However, this strategy only works for the *modified* lines of code -- eg. the `+...` in diffs, and not the `-...` in diffs. The reason is that `git blame --reverse` can't track a line of code's location past the commit that it was removed in.
To permit comments that are left on lines of code that are removed to appear correctly in the UI, a separate approach is required for those comments. This PR performs two major changes, which have been complex to figure out, but are reasonably easy to understand:
- When a comment is placed on a removed line in a PR, perform a `git blame --reverse` from the PR's base to the currently viewed commit, and use this information to record in the comment:
- the **last commit that the line of code existed in** (stored in the `commit_sha` field)
- the **line of code as of that commit** (stored in the `line` field, negative, to indicate that the comment is on a removal).
- the **patch** where the comment was placed (stored in the field `patch`); existing functionality unchanged in this PR
- When viewing any diff in the PR, for each comment on a removal, perform a diff from the `commit_sha` (last commit that the line of code existed in) to the current commit being viewed, and verify that within that diff the left-hand-side line removal still exists at the same line of code in the diff, by comparing the current diff with the stored patch.
- If present, place the commit in the UI at the line number.
- If the line of code no longer exists in the diff at that point (for example, it was removed, commented upon, and then re-added in a later commit), then the comment is considered outdated and isn't displayed.
The algorithm used for marking a comment as "outdated" is also updated to use this approach.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12092
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
Fixes https://codeberg.org/forgejo/forgejo/issues/12082 by reverting commit dd968f147d.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
(can be removed for JavaScript changes)
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [ ] `make pr-go` before pushing
### Tests for JavaScript changes
(can be removed for Go changes)
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
*The decision if the pull request will be shown in the release notes is up to the mergers / release team.*
The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12088
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Henry Catalini Smith <henry@catalinismith.se>
Co-committed-by: Henry Catalini Smith <henry@catalinismith.se>
Currently, forgejo does not support syntax highlighting code-blocks that have comma separated attributes after the language. This is a pattern sometimes seen in Rust code blocks, with tests like this:
\`\`\`rust
#[test]
fn run_this_test() { /* ... */ }
\`\`\`
\`\`\`rust,ignore
#[test]
fn skip_this_test() { /* ... */ }
\`\`\`
Currently, forgejo only does syntax highlighting in the first case:
```rust
#[test]
fn run_this_test() { /* ... */ }
```
```rust,ignore
#[test]
fn skip_this_test() { /* ... */ }
```
An example of this causing problems can be seen in this commit (5be9c5b7d2) causing the following issue (https://codeberg.org/zesterer/ariadne/issues/188).
This PR fixes fixes the second case not getting proper syntax highlighting.
Co-authored-by: TurtleArmy <44322335+TurtleArmyMc@users.noreply.github.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12056
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Ellen Εμίλια Άννα Zscheile <fogti@noreply.codeberg.org>
Co-authored-by: TurtleArmy <turtlearmy@noreply.codeberg.org>
Co-committed-by: TurtleArmy <turtlearmy@noreply.codeberg.org>
This is hopefully the final part of PR #4767, rebased and squashed.
More thorough federation tests are at https://code.forgejo.org/forgejo/end-to-end/pulls/1276 but the mock has been extended to hopefully cover a good chunk as well.
Co-authored-by: Gergely Nagy <forgejo@gergo.csillger.hu>
Co-authored-by: Michael Jerger <michael.jerger@meissa-gmbh.de>
Co-authored-by: zam <mirco.zachmann@meissa.de>
Co-authored-by: Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10380
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: famfo <famfo@famfo.xyz>
Co-committed-by: famfo <famfo@famfo.xyz>
