Requires that all outbound network connections made by migrations & mirroring be performed over encrypted communication channels. As those communication channels (HTTPS and SSH) have their own built-in protections against DNS rebinding (TLS certificate validation and SSH host key validation), DNS rebinding attacks become ineffective as a means to redirect Forgejo to pull-from or push-to unexpected remotes with mirrors.
`[migrations].ALLOW_UNENCRYPTED` is a new setting which defaults to false, and can be set to `true` to restore access to unencrypted remotes. The risks associated with it are documented in `app.example.ini` and will be transferred into the docs repo.
**Breaking:** Existing mirrors configured against `http://` and `git://` protocols will fail, with "unencrypted transfer protocol, remote cannot be validated". It is recommended that those mirrors be changed to encrypted protocols. `[migrations].ALLOW_UNENCRYPTED` may be set to `true` to revert to the previous behaviour, accepting the risks documented here.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13490
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This PR replaces the existing `skip-to` approach for rendering large diffs with paging over the changeset of a git diff, enabling a more efficient way to render the compare view for PR, compare, and commit views.
## Problem
When a user opens a compare page for a large range of commits, Forgejo calculates the entire diff (potentially multiple times) Which consumes significant CPU time on the server and produces an enormous amount of HTML for the browser to parse and display. At some point the page becomes unusable or fails to load entirely.
The previous `skip-to` approach attempted to mitigate this by allowing the user to jump to a specific file or section within the diff. However, this still required the diff process and prepare the full changeset up to that point. It did not fundamentally bound the amount of work done per request: the server could still spend excessive time calculating the Diff. As a result, `skip-to` was not a reliable solution for very large diffs.
TLDR; `--skip-to` is an output-ordering/presentation option, not a performance optimization. [git docs](https://git-scm.com/docs/diff-options#Documentation/diff-options.txt---skip-tofile)
## Solution
This change introduces paging over the changeset of a git diff. Rather than relying on `skip-to`, the diff is now split into manageable pages. Only a subset of the changed files is fetched, highlighted, and rendered at a time.
### 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.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Security features
- [PR](https://codeberg.org/forgejo/forgejo/pulls/12808): <!--number 12808 --><!--line 0 --><!--description ZG9uJ3QgcmVuZGVyIGxhcmdlIGNvbXBhcmUgZGlmZg==-->don't render large compare diff<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12808
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Fixes#13440.
## Testing
Manual testing was performed with an automated reproduction. Attached `main.go` program was used to test go-git interaction with a server using `START_SSH_SERVER = true`. This test app uses go-git to perform an ssh-based push, which replicates the behaviour described in #13440. After dropping main.go into an empty directory, the test app can be run with:
```
go mod init repro
go mod tidy
REPRO_REPO_URL=ssh://mfenniak@localhost:2222/mfenniak/test.git go run main.go
```
When the fix is **not** present, the database/repository desync warning is shown after running the test command, which indicates that the hooks were interrupted and did not update the database state of the `main` branch:
When this fix **is** present, the database/repository desync warning does not appear, which indicates that the hooks were not interrupted. Repeated test in both modes 3x times to ensure there were no race conditions involved.
While it would be possible to incorporate this test into Forgejo's integration testing suite, it would require adding a dependency to `go-git`. As Go doesn't have a concept of test-only dependencies, that would make `go-git` a dependency... which we just recently removed by removing it from Forgejo Runner's jobparser library. It would make `go-git` available for accidental incorporation into Forgejo's code, where it does not behave the same as Forgejo's internal git CLI libraries.
### 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...
- [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/13443
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Continuation of #10598Closes#7254
Implement repo granular watch selection
I adopted a lot of the frontend from #10598 while redoing the entire back-end. I propose this new model:
- Remove the WatchMode enum.
- Add the WatchSelection struct, which represents the granular selection. Notice that there is no `watching` bool. I tried very hard to keep the structure as simple and redundancy-free as possible. Therefore, people not watching a repo at all either don't have a watch record or one with an entirely unselected WatchSelection struct.
- Add the WatchSource enum. It replaces the WatchMode enum and has a single purpose: determine whether a watch was explicitly or automatically initiated.
Notice that replacing this
```go
And("`watch`.mode<>?", WatchModeDont).
```
with this is correct:
```go
And(
builder.Or(
builder.Eq{"`watch`.watch_selection_issues": true},
builder.Eq{"`watch`.watch_selection_pull_requests": true},
builder.Eq{"`watch`.watch_selection_releases": true},
),
).
```
That's because there are four modes: dont, none, auto and normal. When `<>` with dont, we look for auto and normal, because there are no records with none. Therefore, the old code looks for records that indicate watching. The code I replaced this with does so, too, just more granular.
Also notice that I've prepared a future `user preset` in a few places. See below for a little more info on that.
## Next PR
I plan to continue working on this. I want to implement a `user preset` option. The user sets that `user preset` in her settings and may use them in any repo.
<details>
- rename account settings to account and notifications
- user preset (always use this preset for newly accessible repos (according to AutoWatchOnChanges and AutoWatchNewRepos))
</details>
## Further PRs
- make api able to granular watch
- move (email) notifications to new notifications tab
Co-authored-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-authored-by: pat-s <patrick.schratz@gmail.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10927
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Adds an extra check to ensure the `keyId` and `actorId` included in signed requests and actor records point back to the originating host.
This check prevents server-side request forgery (SSRF) attacks where a carefully crafted request could be used to trick a federation server into making requests to arbitrary hosts and ports.
Further refactors can make these checks more robust, but would better fit after other existing refactor PRs are merged.
Related: https://codeberg.org/forgejo/forgejo/issues/11779
### 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
- [ ] 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/11795
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
follow-up to #13306
## 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.
- [ ] 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.
- [ ] 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/13340
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
fixes#13305
### 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...
- [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.
- [ ] 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/13306
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
The rationale for this decision is the apparent confusion regarding the number of issues that were presented.
For a query such as `foo bar` the natural expectation would be results that contain both `foo` and `bar` not either `foo` or `bar`.
However, for a query such as `+foo bar` the result MUST contain `foo`, while `bar` affects only the scoring of issues.
A downside of such an approach is the inability to fully represent `foo OR bar` as the current implementation imposes that the query must contain atleast one MUST (or MUST NOT) term.
closes#12935
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13025
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Test failure: https://codeberg.org/forgejo/forgejo/actions/runs/174007/jobs/2/attempt/1 Have only observed once and can't reproduce locally, but likely caused by #13263 / #13289.
```
--- FAIL: TestGetDiffPreview (0.20s)
--- FAIL: TestGetDiffPreview/empty_branch,_same_results (0.02s)
diff_test.go:132:
Error Trace: /workspace/forgejo/forgejo/services/repository/files/diff_test.go:132
Error: Received unexpected error:
unable to run diff-index pipeline in temporary repo: exec: canceling Cmd: invalid argument
Test: TestGetDiffPreview/empty_branch,_same_results
FAIL
```
pidfd_open is [documented](https://www.man7.org/linux/man-pages/man2/pidfd_open.2.html) to return the EINVAL (invalid argument) error if the pid is not valid, which could happen if the process terminates itself before Forgejo sends `SIGTERM`. This PR adds a check for that error and stops `Cancel`, with a minor refactor to avoid code duplication.
### 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
- [ ] 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/13293
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
#13263 unintentionally prevents Forgejo from building on non-Linux UNIX OSes. This PR adds a second graceful shutdown implementation which uses repeated signalling of the subprocess to detect if it is still running. This additional implementation is used for non-Linux UNIX platforms. It is also a fallback when Linux returns `ENOSYS` which should be the case in Linux kernels older than 5.3.
## Testing
- Validated build fails on a FreeBSD VM w/ head `forgejo`
- `FreeBSD freebsd-test 15.1-RELEASE FreeBSD 15.1-RELEASE releng/15.1-n283562-96841ea08dcf GENERIC amd64`
- Validate build succeeds and process tests pass on FreeBSD VM w/ this branch:
```
mfenniak@freebsd-test:~/Dev/forgejo $ GO_TEST_PACKAGES=forgejo.org/modules/process make test-backend
Running go test with -tags 'sqlite sqlite_unlock_notify'...
ok forgejo.org/modules/process 5.690s
```
- Note: other some other test modules do not pass like `forgejo.org/modules/log`, but this seems consistent with `forgejo` head. As we don't have a baseline for expectations here, this isn't an issue that can be addressed in this PR.
- Fallback for pre-Linux 5.3 has been tested on a Debian Buster install, kernel 4.19.0, and confirmed `forgejo.org/modules/process` test success, and test failure without the fallback path.
### Tests for Go 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
### 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.
- Adjustment to unreleased bugfix.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13289
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
By default, when you create an `exec.CommandContext` in Go, when the context is cancelled or timed out, Go will SIGKILL the subprocess which does not allow the subprocess to perform any cleanup work. As some git operations in Forgejo will be linked to the current HTTP context, and the current HTTP context will be closed when the HTTP client disconnects, this can theoretically cause SIGKILLs when locks are being held.
This PR contains two fixes:
- Change the behaviour of all subprocesses that Forgejo creates to use a graceful termination, SIGTERM'ing the process, and then SIGKILL'ing it after a timeout.
- Fixes a bug where previously all subprocesses are setup in a process group (by 1d04e8641d), but they were never *killed* in a process group, preventing child processes from receiving these termination signals. Both SIGTERM and SIGKILL are sent to the process group.
Introduces new configuration setting which controls subcommand graceful timeouts. If a subprocess does not shutdown from SIGTERM within `[server].SUBPROCESS_TERMINATE_GRACE` (default 5 seconds), then an error will be logged, and the process will be SIGKILL'd. The error is:
```
2026/07/01 08:31:20 modules/log/init.go:38:func2() [E] Subprocess pid=3790963 (/nix/store/c0277k5giric1mn9dklllavbzvxl6hzb-git-2.53.0/bin/git blame --porcelain 8ee7e7c32b -- options/locale/locale_en-US.ini) failed to terminate from SIGTERM after grace period 0s, and was sent SIGKILL. The subprocess termination may leave incomplete state on the server, such as lock files. `[server].SUBPROCESS_TERMINATE_GRACE` can be changed to give subprocesses more time to cleanly exit.
```
(This was produced on a live server by setting SUBPROCESS_TERMINATE_GRACE to zero seconds, loading a page that does a `git blame`, and hitting escape in the browser quickly.)
### Tests for Go 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
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- Will create a documentation PR with new config settings.
- [ ] 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/13263
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
There is no absolute threshold to decide a cache is slow. Report the time it took to run the test and let the user decide if this is too slow or not.
Closesforgejo/forgejo#5846
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13256
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
When downloading archives Forgejo uses git cat-file to resolve a ref name (e.g. branch or tag) to a commit id. There were a check if the input already was a valid full commit id, sadly without a check if the commit exists. Therefore, I removed this "early return".
I noticed this while trying to download archives from Codeberg and the there was just a timeout after 10min. Upon trying this on a self-hosted instance I saw the error in the logs, which led me to his fix.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12963
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
fixes#12108
docs PR: https://codeberg.org/forgejo/docs/pulls/1893
### 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
- [x] 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.
*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.
## Testing
Related PRs:
- https://code.forgejo.org/forgejo/end-to-end/pulls/2063
- https://code.forgejo.org/forgejo/oci-mirror/pulls/69
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12200
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Similar to #13129, fixing a few places where `[migrations].ALLOWED_DOMAINS`, `...BLOCKED_DOMAINS`, and `...ALLOW_LOCALNETWORKS` are not implemented in external migrations, creating SSRF risks.
### Tests for Go 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
### 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/13184
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
System administrators can determine which hosts can be accessed by Forgejo for git mirroring via the config settings `[migrations].ALLOWED_DOMAINS`, `...BLOCKED_DOMAINS`, and `....ALLOW_LOCALNETWORKS`. However, there were edge cases where these settings were ineffective:
- Config entries were only checked when mirrors were initially configured, and not later when a mirror pull/push occurred. DNS changes, or server configuration changes, could cause a previously safe URL to no longer be safe. These settings are now re-checked on every mirror pull/push.
- When accessing a git repository over http, git would follow HTTP redirects, allowing a violation of the `[migrations]` config entries. The git option `http.followRedirects=false` is now set to prevent git from following these redirects.
- When performing Git LFS synchronization, it was assumed that the git URL passing migration URL checks was sufficient protection. Specifically for pull mirrors which allow the usage of "Advanced settings" -> "LFS endpoint" to configure a separate LFS endpoint, these settings were never enforced, and for all mirrors they would have been subject to not being rechecked if the DNS had changed since configured. LFS mirroring now uses an HTTP client that enforces the `[migrations]` config settings.
**Breaking:** If a repository mirror that depends on HTTP redirects will no longer work.
For example, if you've mirrored a repository from a Forgejo instance at `https://example.com/OWNER/REPO.git` and the repository or owner are renamed, Forgejo will provide an HTTP redirect that continues to allow git operations to work at the old URL. Due to this redirect prohibition, this mirror will no longer work.
### 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
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- It is not currently documented that the `[migrations]` settings are used to control access to mirrors. Forgejo advises it when an error occurs, but it could be added: https://codeberg.org/forgejo/docs/pulls/2023
- [ ] 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/13129
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
When a user has a theme in the DB that is not among the themes in the configuration, the following happens to this user's UI:
Image: https://codeberg.org/attachments/bf8d4ff1-8216-4df5-ab90-8dc7e03784d9
The workaround is to manually go to Appearance settings and update the theme.
This can happen if the theme was removed from the server config. For example, admins don't want to have it anymore. Maybe it even was the default theme, which is being saved in the DB during sign up.
It will be useful for Forgejo if we, for example, want to separate colorblind them variants from the actual themes, or if we ever want to remove the Gitea themes. Rel: https://codeberg.org/forgejo/forgejo/pulls/13054.
And instance admins will also find it useful to not have to manually update the DB in case they want to get rid of some custom theme.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13110
Reviewed-by: Robert Wolff <mahlzahn@posteo.de>
In #9638, repository and user avatars had an EXIF removal capability added to them. Unfortunately, this capability was added with an AGPL licensed library, exif-terminator, which is incompatible with Forgejo's license. This was not detected by license check automation at the time.
This PR removes the capability in order to fix the license compatibility. The `forgejo doctor avatar-strip-exif` is retained with a warning output that it is not supported.
Reopens: forgejo/forgejo#9608
## 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.
- [ ] 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/13105
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Fixes#12564. Fixes#8951.
This introduces a new setting, `ADD_MEMBERS_BY_INVITATIONS`, which is turned off by default.
When turned on, adding a user to a team issues an invitation instead of adding them directly to the team.
A prerequisite for this work was to be able to link invitations to existing users (so far, they were only associated to an email address, since those invitations were meant to be issued to users who didn't have an account yet).
---
I plan to work on the following improvements, which I propose to do in separate PRs given that this one is already a bit big:
* generate an in-app notification for the invited user
* advertise the invitation to the invited user from the org page as well (#12120)
* show the list of invited users in the list of organization members (not just on the team page)
and various other improvements to invitations (#12570, #12716).
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12845
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This PR fixes some minor issues with the swagger API documentation. These issues are:
1. The description for the `template` parameter for the `GET /repo/search` endpoint is incorrect. It says this:
```
include template repositories this user has access to (defaults to true)
```
When the parameter actually functions like this:
```
show only template, non-template or all repositories (defaults to all)
```
2. The the `gitignores` option in the `POST /user/repos` endpoint JSON object has a description that doesn't say how `gitignore` names should be separated. It's like this:
```
Gitignores to use
```
When it should be something like this:
```
Gitignores to use, separated by commas
```
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13082
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
The routines for the removal of `ActionTask` expect that each `ActionTask` has logs attached and that `LogFilename` isn't empty. However, that is not always the case. For example, Forgejo can resolve jobs without dispatching them to a runner. In that case, a placeholder task is created without logs and `LogFilename`. The log removal routines simply concatenate the path of the log storage directory and `LogFilename` and try to delete that without verifying that `LogFilename` is present. Consequently, they try to remove the log storage directory. In most cases, that causes an error because the directory contains some files. To prevent that from happening, the log removal routines no longer allow empty filenames. And it is checked whether a task has logs before invoking them.
### Tests for Go 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...
- [ ] `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/13040
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Otherwise the newly introduced test from #12335 fails on platforms which do not have git sha256 yet
## 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.
- [ ] 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
- [ ] 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/13018
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
To efficiently accept a number of keys containing key IDs, we need a
closure argument to jwt.ParseWithClaims()
This implementation provides management of a number of keys by indexing
them by either algorithm (symmetric keys) or algorithm & key ID
(asymmetric keys). A ParseWithClaims() type method then provides lookup
of the right key for a passed token.
As an additional security improvement, we also add signing methods for
which we have keys in the verifier to the jwt parser, such that attempts
to use a keys with unknown signing methods can be refused early on in
the parsing process.
With this in place, the typical flow to use a number of keys for
verification is:
- in the settings parser, initialize a key config:
keyCfg, err := loadKeyCfg(rootCfg, "section", "pfx", "ALG", "privkey")
- in the settings parser or in a module init, initialize a signingkey +
verifier pair from the confic:
signingKey, verifier, err = jwtx.Init(&keyCfg)
- Use signingKey.JWT(claims) to create signed tokens
- Use
_, err := verifier.ParseWithClaims(token, &claims)
to verify the token with any key in verifier and return the parsed
claims.
We add parsing of [pfx]KEYS_ACCEPTED into a new intermediate form
jwtx.VerificationKeyCfg.
The format of [pfx]KEYS_ACCEPTED is <alg>:[<base64>|<uri>] ... to accept
any number of keys to verify signatures.
As before, for now, only file: scheme uris are supported, but the path
is passed to filepath.Glob(), such that patterns can be used to easily
configure a larger number of keys, for example
[pfx]KEYS_ACCEPTED = RS256:file:module/pub/*.pem HS256:file:module/old.key
For asymmetric keys, private PEM key files are accepted in addition to
public key files to simplify key management.
This commit momentarily adds dead code, which for now is only used in
tests. The actual use case is going to be added in a follow up, but this
commit is still kept separate for clarity.
Found issues during the process of invalidation of a multiline comment (link to #12582):
* Update a line in the middle of the comment
* Update/Delete the last line of the comment
No problem with:
* Deleting a line in the middle of the comment
* Update/Delete the first line of the comment
I added all these cases in the pull_review_test.go
### Tests for Go changes
- I added test coverage for Go changes...
- [X] in their respective `*_test.go` for unit tests.
- I ran...
- [X] `make pr-go` before pushing
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12950
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
This PR pertains to the client-side validation of the Website input on user, repo, and org profiles. #12962 extends `[service].VALID_SITE_URL_SCHEMES` to cover Website fields on repo and org profiles, where before that config key only applied to the one on user profiles. If that change merges, it will then be possible to construct an HTML [`pattern`](https://developer.mozilla.org/docs/Web/HTML/Reference/Elements/input#pattern) attribute for general use on any Website form input that the server validates this way, thus enabling browsers to catch errors early relating to URL scheme confusion.
This PR (1) introduces such a `pattern` attribute, and (2) adds a new UI note to make clear to users which URL schemes are permitted. This change helps explain the browser's otherwise cryptic error messages regarding pattern mismatch, while also letting users know what URI schemes the Forgejo instance supports as Website links (e.g. gemini:// URLs).
This MUST NOT merge before #12962. To do so would introduce a regression wherein the UI may suggest and validate a different set of allowed URL schemes than the server actually permits.
See also #5519
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12991
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Turns out this was a one-line fix for each affected field (change the binding from `ValidUrl` to `ValidSiteUrl`), but the tests are rather verbose. The tests are, however, each a simple flow of Create Thing > Try HTTP Website > Try Different Website (notice failure) > Try Different Website With New Config (notice success). I wrote this PR by adding failing tests first, then making the change, for each affected field.
Not sure if this should be "feat:" or "fix:" tbh. I figured "fix:" for this PR since IMO the expected behavior is for `VALID_SITE_URL_SCHEMES` to apply in each of these cases, not only for user profiles via the UI form. (Later changed to "feat:" at @limiting-factor's suggestion, based on the observation that this change extends documented behavior.)
This PR deals with the server-side validation only. #12991 covers client-side validation (deriving a `pattern` attribute from `VALID_SITE_URL_SCHEMES`, etc.)
Closes#5519
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12962
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Adds an option "force_overwrite_new_branch" when posting to
/repos/{owner}/{repo}/contents to modify multiple files in a repository.
When user provides both "branch" and "new_branch" options, and
"new_branch" already exists, the "force_overwrite_new_branch" option
allows the user to overwrite the existing branch. Under the hood this
amounts to a "git push --force".
[Issue #12600](https://codeberg.org/forgejo/forgejo/issues/12600)
### 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.
- [ ] I did not document these changes and I do not expect someone else to do it.
- [x] API swagger docs updated
### 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.
Co-authored-by: Rob Gonnella <rob.gonnella@papayapay.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12663
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Closes https://codeberg.org/forgejo/forgejo/issues/6093
This PR adds support for **multi-line review comments** on pull requests, allowing reviewers to select a range of lines in diffs instead of only a single line — similar to GitHub's implementation.
### Tests for Go changes
- I added test coverage for Go changes...
- [X] in their respective `*_test.go` for unit tests.
- [X] `make pr-go` before pushing
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12582
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
First, why was this header here in the first place? Cloudflare!
Cloudflare had a optimization setting called "auto-minfy" and would
minify HTML,JS,CSS - this included removing extra whitespaces from
`<code>` elements. That's a problem because files are shown per-line
with a `<code>` element and thus results in indentation being completely
gone. Gitea added a FAQ entry for this [1], but on the same day decided
to add the workaround in Gitea, the `no-transform` header [2].
I can't find a reference of this option and some posts suggests it's
been removed. Thus it no longer serves a need to be present in Forgejo.
That wasn't my intentional motivation to remove this. This header is
also causing that HAProxy will not compress responses [3] from Forgejo
which is not ideal for Codeberg, this behavior cannot be turned off or
be worked around.
Potential risk, some other CDN or some other Cloudflare option might
still do this removal of whitespace in `<code>` HTML tags, it seems
better to disable the feature than to have Forgejo add a header which is
also causing other side-effects. I'm not aware of this another CDN of
Cloudflare option so I don't want to mark it as breaking.
[1]: https://github.com/go-gitea/gitea/pull/20430
[2]: https://github.com/go-gitea/gitea/pull/20432
[3]: https://docs.haproxy.org/3.3/configuration.html#:~:text=the%20response%20contains%20the%20%22no-transform%22%20value%20in%20the%20%22Cache-control%22%20%20%20%20%20header
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12905
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Thanks to forgejo/forgejo!10397 (by @voidcontext), the binary called on git hooks can now be dynamically set.
**This means that we can now run tests without needing to run `make gitea` first**! No more `Could not find gitea binary` or head-banging, when one forgets to re-compile it 🎉
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12855
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Since forgejo/forgejo!10397 and forgejo/forgejo!12335 have landed, there shouldn't be a need for the `hooks` directory in each repository.
This PR cleans up the `tests/gitea-repositories-meta/*` testdata.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12824
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Extracted from #12824 on suggestion of @limiting-factor; refactored after @Gusted pointed out forgejo/forgejo!12335.
Behavior change: previously a missing `hooks` folder in a repository tree (should not happen before forgejo/forgejo!12335) would return a 500 on `/api/v1/repos/%s/hooks/git`. It now returns a 200, with the same reply as an empty `hooks` folder.
Test has been added to ensure correct handling of missing `hooks` folder and of its creation if necessary.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12833
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Prevent examples hooks, description file, and other files from the default template to be created during git init.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12335
Reviewed-by: Otto <otto@codeberg.org>
A handful of routes, described in this PR as "mixed routes", are currently accessible by both web-based sessions and authenticated API users. The goal of this PR is to allow access to these routes for Authorized Integrations as well, bringing them to full API compatibility (to my knowledge) with other authentication methods. These routes are impacted:
- `/{username}/{repo}/raw/*`
- `/{username}/{repo}/archive/*`
- `/{username}/{repo}/releases/download/{vTag}/{fileName}`
- `/{username}/{repo}/attachments/{uuid}`
- `/attachments/{uuid}`
The major work in this PR was to refactoring the existing authentication methods so that "path based matching" that they were currently doing was no longer required, as I didn't want to introduce that into Authorized Integrations. All the path based matching is removed in this PR, and authentication methods are enabled entirely by the middleware applied to their 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
- 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
- [ ] 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/12776
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Since the final size is already known, no need to `ReadAll` a `LimitedReader`: directly `ReadFull` a properly sized buffer.
Tests are already present in `blob_test.go` (a failure can be triggered by creating a smaller `buf`).
`go test -run=TestBlob_Data -bench=Blob_Data -benchmem` before:
```
Benchmark_Blob_Data-18 43964 28727 ns/op 1373 B/op 11 allocs/op
```
After:
```
Benchmark_Blob_Data-18 41308 27679 ns/op 846 B/op 10 allocs/op
```
🎉 one allocation spared!
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12795
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
The goal is to enable access to Git LFS resources with Authorized Integrations JWTs.
Blocker that needed to be resolved is that adding the `AuthorizedIntegration` auth method would conflict with the LFS tokens, which are handed out during git ssh clones to allow access to LFS resources -- `AuthorizedIntegration` would mark these as `AuthenticationAttemptedIncorrectCredential`, and therefore the requests would 401 before they got to the LFS-specific token validation routines. The fix is to move LFS token authentication into an authentication group so that it could be resolved at the same time as the authorized integration, rather than doing it inside the LFS server routines.
Refactors for LFS tokens are covered by refreshed test automation. Authorized integrations LFS Access has been manually tested, and will be further covered in an end-to-end integration test (https://code.forgejo.org/forgejo/end-to-end/pulls/1954).
## 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.
- [ ] 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/12725
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
