Forgejo's UI claims that whitespace is removed from the beginning and the end of the values of Forgejo Actions variables and secrets. However, that is not correct. The entered values are stored as-is. Only CRLF is replaced with LF, which is also the desired behaviour.
This PR changes the incorrect text which is also no longer displayed as placeholder but as a proper help text below the input fields. Furthermore, tests were added to verify the behaviour.
While adding tests, I discovered and fixed another inconsistency. Depending on whether secrets were managed using the UI or the HTTP API, they were treated differently. CRLF in secrets entered in the UI was correctly replaced with LF while secrets created using the HTTP API kept CRLF.
Fixes#11003.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11052
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Co-committed-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
This PR fixes a number of typos throughout the entire repository. Running https://github.com/crate-ci/typos and then changing all occurrences that I naively deemed "safe enough".
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10753
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Christoph Mewes <christoph@kubermatic.com>
Co-committed-by: Christoph Mewes <christoph@kubermatic.com>
Changes foreign key database inconsistency handling so that inconsistent records are automatically deleted with an administrator warning during migration. As noted in discussion: https://codeberg.org/forgejo/discussions/issues/385#issuecomment-9175566
Intended for backport to v14 to eliminate pain during upgrades.
Because these migrations are now deleting data, rather than allowing the administrator to do it, all migrations have been covered with an integration test that verifies expected data is deleted. This is particularly interesting with nullable fields.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. 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
- 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 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
- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10568
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
Currently `DeriveKey` is called every time that a secret must be encoded/decoded. Since this function is deterministic, its result can be cached to allow a 250x speedup (the original took less than half a microsecond, so this more of a micro-optimization...).
```
go test -bench=.
goos: linux
goarch: amd64
pkg: forgejo.org/modules/keying
cpu: Intel(R) Core(TM) Ultra 5 125H
BenchmarkExpandPRK-18 2071627 564.2 ns/op
BenchmarkExpandPRKOnce-18 541438192 2.206 ns/op
PASS
ok forgejo.org/modules/keying 2.369s
```
## Other changes
- Since the keys can be constructed once, it simplifies a bit the callsites (`keying.TOTP.Encrypt(...)` instead of `keying.DeriveKey(keying.ContextTOTP).Encrypt(...)`)
- All `Encrypt`/`Decrypt` calls will panic forever if called before `Init` has been called (current it panics as long as `Init` has not been called)
- Calling `Init` twice with different keys will trigger a panic (currently racy)
- Calling `Decrypt` with a short ciphertext does not panic anymore (like when calling with long-enough garbage)
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10114
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>
For both `forgejo_version` and `version`, updates the migration to require one-and-only-one record in the table with `id = 1`. Intended to detect situations such as #9771 and block Forgejo running, preventing a long period of missing migration applications due to data corruption.
In addition to automated tests, I manually corrupted my `forgejo_version` and `version` table to test the behaviours.
Backports to v11 & v13 planned, although both will require manual backports due to #9561 reorganizing these files.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. 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
- 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 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
- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
- [PR](https://codeberg.org/forgejo/forgejo/pulls/9773): <!--number 9773 --><!--line 0 --><!--description c3RyaWN0IGVycm9yIGhhbmRsaW5nIG9uIGNvcnJ1cHRlZCBEQiBtaWdyYXRpb24gdHJhY2tpbmcgdGFibGVz-->strict error handling on corrupted DB migration tracking tables<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9773
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
Integration tests and new Forgejo installations work in a manner that I didn't expect, and didn't come up during testing of #9561.
- They run the migrations (gitea migrations -> forgejo migrations legacy -> forgejo migrations).
- The gitea & forgejo migrations detect if the `version` and `forgejo_version` table are **empty**, because they are on an empty DB and the tables were just created
- When they hit this case, they pretend all the migrations are applied by setting the version to the highest version number
- And then they rely on the DB engine executing `SyncAllTables` to actually create the schema.
The new `forgejo_migrations` module was not doing this. So, it would attempt to execute migrations and find that dependent database tables didn't exist, causing unexpected errors in running the integration tests (or starting forgejo on an empty database): https://codeberg.org/forgejo/forgejo/pulls/9397#issuecomment-7745969
This fixes the issue by following the same pattern in `forgejo_migrations`; relying on `SyncAllTables` for initial schema creation.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. 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
- 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 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
- [x] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9708
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
