Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add form_secret_path config option #18090

Merged
merged 4 commits into from
Feb 25, 2025
Merged

Conversation

V02460
Copy link
Contributor

@V02460 V02460 commented Jan 15, 2025

I was told about another config option with a secret, so I got form_secret a companion: form_secret_path

This PR makes NixOS and Kubernetes users a little bit happy. Includes docs and tests.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct
    (run the linters)

@V02460 V02460 requested a review from a team as a code owner January 15, 2025 19:14
@github-actions github-actions bot deployed to PR Documentation Preview January 15, 2025 19:15 Active
@devonh
Copy link
Member

devonh commented Feb 24, 2025

The changes look good to me @V02460!
Thank you for keeping on with these secret path changes.

If you resolve the conflict I will take another look and we should be able to merge this.

@github-actions github-actions bot deployed to PR Documentation Preview February 25, 2025 13:13 Active
devonh pushed a commit that referenced this pull request Feb 25, 2025
Adds the `--no-secrets-in-config` command line option that makes Synapse
reject all configurations containing keys with in-line secret values.
Currently this rejects

- `turn_shared_secret`
- `registration_shared_secret`
- `macaroon_secret_key`
- `recaptcha_private_key`
- `recaptcha_public_key`
- `experimental_features.msc3861.client_secret`
- `experimental_features.msc3861.jwk`
- `experimental_features.msc3861.admin_token`
- `form_secret`
- `redis.password`
- `worker_replication_secret`

> [!TIP]
> Hey, you! Yes, you! 😊 If you think this list is missing an item,
please leave a comment below. Thanks :)

This PR complements my other PRs[^1] that add the corresponding `_path`
variants for this class of config options. It enables admins to enforce
a policy of no secrets in configuration files and guards against
accident and malice.

Because I consider the flag `--no-secrets-in-config` to be
security-relevant, I did not add a corresponding `--secrets-in-config`
flag; this way, if Synapse command line options are appended at various
places, there is no way to weaken the once-set setting with a succeeding
flag.

[^1]: [#17690](#17690),
[#17717](#17717),
[#17983](#17983),
[#17984](#17984),
[#18004](#18004),
[#18090](#18090)


### Pull Request Checklist

<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->

* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
  - Use markdown where necessary, mostly for `code blocks`.
  - End with either a period (.) or an exclamation mark (!).
  - Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct
(run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
@devonh
Copy link
Member

devonh commented Feb 25, 2025

Looks like there is another conflict now after merging in the --no-secrets-in-config PR.
Hopefully that's the last thing

@github-actions github-actions bot deployed to PR Documentation Preview February 25, 2025 19:17 Active
@devonh devonh merged commit 131607e into element-hq:develop Feb 25, 2025
41 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants