docs: document remaining undocumented [global] config keys#645
Conversation
Five `gcfg`/`mapstructure`-tagged keys on `Config.Global` either had no operator-facing documentation at all or were only referenced obliquely in passing prose, even though the parser has accepted them for several releases. Closes the docs side of the drift class flagged by TestConfigGlobalKeysAreDocumented. - docs/CONFIGURATION.md: add `notification-cooldown` (notification deduplication window, default 0 = disabled) and `smtp-tls-skip-verify` (default false) to the [global] reference, both with the security framing inline so operators don't have to cross-reference the source. - docs/webhooks.md: expand the "Global Settings" code block to cover `webhook-webhooks`, `webhook-trusted-preset-sources`, `webhook-preset-cache-dir`, and `webhook-allowed-hosts` with their defaults, and add an explicit INI-vs-Docker-labels callout — only `webhook-webhooks` is exposed via labels, the SSRF-sensitive globals are INI-only per #486 / #620. - docs/TROUBLESHOOTING.md: add a dedicated "SMTP TLS verification trade-off" section that spells out exactly what `smtp-tls-skip-verify = true` does to the dialer, when it's appropriate (test environments, internal CAs) and when it isn't (public relays, credentials shared with downstream systems), with recommended alternatives in preference order. Refs #635, #621, #604. Signed-off-by: Sebastian Mendel <[email protected]>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
github-actions
Bot
added
the
documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request documents several previously undocumented global configuration keys, including notification-cooldown, smtp-tls-skip-verify, and various webhook settings, while adding a security-focused troubleshooting section for SMTP TLS verification. Feedback from the review identifies technical inaccuracies regarding which notification types support cooldowns and which webhook settings are compatible with Docker labels. Additionally, the reviewer suggested using more consistent allow-list terminology and corrected the recommended file path for mounting CA certificates when using update-ca-certificates.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #645 +/- ##
=======================================
Coverage 87.86% 87.86%
=======================================
Files 88 88
Lines 10898 10898
=======================================
Hits 9576 9576
Misses 1081 1081
Partials 241 241
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates Ofelia’s operator documentation to cover several [global] configuration keys that are accepted by the parser but were previously undocumented or only mentioned indirectly, addressing docs-vs-code drift flagged by TestConfigGlobalKeysAreDocumented.
Changes:
- Document
notification-cooldownandsmtp-tls-skip-verifyindocs/CONFIGURATION.md, including a security warning for SMTP TLS verification skipping. - Expand
docs/webhooks.mdwith additional webhook global settings and an explicit INI-vs-Docker-labels callout for SSRF-sensitive keys. - Add a dedicated troubleshooting section explaining the security trade-offs of
smtp-tls-skip-verify, and record the docs updates inCHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/webhooks.md | Documents additional webhook global keys and clarifies INI vs Docker-label availability. |
| docs/TROUBLESHOOTING.md | Adds an in-depth section on smtp-tls-skip-verify risks and safer alternatives. |
| docs/CONFIGURATION.md | Adds [global] reference entries for notification-cooldown and smtp-tls-skip-verify. |
| CHANGELOG.md | Notes the documentation drift fixes and newly documented keys. |
Security review of #645 flagged three docs gaps: - webhook-trusted-preset-sources lacked explicit SSRF allow-list framing — operators could read the example as a generic glob list and miss the implication that bare `*` patterns enable Ofelia to reach cloud-metadata endpoints / internal services on behalf of the preset string source. - webhook-preset-cache-dir documented the malicious-container threat but not the symmetric host-side threat: a world-writable or shared cache directory lets anyone with write access plant preset files that Ofelia will load and execute as middleware config. - TROUBLESHOOTING.md's smtp-tls-skip-verify example trailed the warning section by ~50 lines and used `${SMTP_PASSWORD}` without a final reminder that the credential is exposed to any MITM on every send. Skim-readers who jumped to the example missed the trade-off discussion. All three are docs-only. Refs #635. Signed-off-by: Sebastian Mendel <[email protected]>
|
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
Summary
Documents five
gcfg/mapstructure-tagged keys onConfig.Globalthat were either entirely undocumented or only mentioned obliquely in passing, even though the parser has accepted them for several releases. Closes the docs side of the drift class flagged byTestConfigGlobalKeysAreDocumented(added in #621).Keys covered:
notification-cooldowncli/config.go:72docs/CONFIGURATION.md[global] referencesmtp-tls-skip-verifymiddlewares/mail.go:27docs/CONFIGURATION.md[global] + dedicated trade-off section indocs/TROUBLESHOOTING.mdwebhook-webhooksmiddlewares/webhook_config.go:72docs/webhooks.mdGlobal Settingswebhook-trusted-preset-sourcesmiddlewares/webhook_config.go:79docs/webhooks.mdGlobal Settingswebhook-preset-cache-dirmiddlewares/webhook_config.go:85docs/webhooks.mdGlobal SettingsAlso adds an explicit INI-vs-Docker-labels callout in
docs/webhooks.mdreiterating that onlywebhook-webhooksis exposed via labels (asofelia.webhook-webhooks); the SSRF-sensitive globals are INI-only per #486 / #620.The
smtp-tls-skip-verifyentry indocs/TROUBLESHOOTING.mdspells out exactly whatInsecureSkipVerify = truedoes to the dialer, when it is acceptable (test environments, internal CAs that cannot be installed system-wide, legacy mail servers as a temporary workaround) and when it is not (public relays like Gmail/SendGrid/SES, credentials that grant write access to mailboxes other systems trust), with recommended alternatives in preference order.Closes #635. Refs #621, #604.
Test plan
go test ./cli/ -run TestConfigGlobalKeysAreDocumented -count=1 -v— passes (the test only walks anonymous/embedded structs today; the directnotification-cooldownfield is covered by the prose addition rather than the reflection sweep, per the existingTODO(#635)in the test).go test ./cli/ -count=1— entirecli/suite passes.go build ./...— clean.golangci-lint run ./...— 0 issues.loadDocs(...)incli/config_drift_test.go.