fix(config): attribute admin system-config writes so they survive restart#104
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #97.
Problem
Env-mapped
system_configrows are re-seeded from their environment variable on every boot for any row whoseupdatedByisNULL. Admin console writes go through the access-token path (auth: 'access'+requireAdmin), which setreq.userbut neverreq.clientId; the controllers derivedupdatedByonly fromreq.clientId, so it was written asnull. The row stayed eligible for env re-seeding, and admin changes (adding an OAuth provider, enabling theoauthlogin method, editing origins/roles) were silently reverted on the next restart, contradicting the documented contract.Change (behavior + docs)
resolveSystemConfigUpdatedBy(req)helper (src/lib/systemConfigActor.ts): prefersreq.clientId(service-token path) and falls back to the acting admin'sreq.user.id(access-token path). BothPATCH /system-config/adminand the/system-config/oauth-providersendpoints use it, so an admin change now setsupdatedByand becomes authoritative.bootstrapSystemConfig.ts: logs awarnwhen a boot overwrites a stored value that differs from the env-derived one (key and env-var name only, no values), so the reseed is no longer silent.docs/configuration.md: the "Environment vs system_config" section (and the top-of-file blockquote) now describe the real, now-guaranteed precedence, including how to hand a row back to env control (clearupdatedByor delete the row).Note on existing rows
Rows already written before this change still report
updatedBy IS NULL, so they remain env-seeded until the next admin write re-attributes them. No data migration is included; that is a deliberate scope choice (call out if a backfill is wanted).Tests
bootstrapSystemConfig.spec.ts): asserts the overwritewarnfires for differing rows and does not fire when the stored value already matches.PATCH /system-config/adminandPOST /system-config/oauth-providersnow assertupsertis called withupdatedBy: 'user-1'(the authenticated admin).Checks
typecheck, lint, config + systemConfig suites all pass. Security review passed (no findings). Changeset included (
minor).