fix(admin): show resolved runtime values on /admin/settings (#7803)#7807
Merged
Conversation
Side-channel resolved+redacted settings alongside raw file blob.
Form view dropdowns and env pill chips reflect actual runtime values
instead of falling back to template defaults. Save round-trip is
unchanged so ${VAR:default} literals stay intact on disk.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure helper that walks the live settings module and replaces known sensitive paths (users.*.password, dbSettings.password, sso.clients[*].client_secret, sessionKey, …) with [REDACTED] sentinel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…7803) Existing 'results' raw-file blob is unchanged so the textarea editor and saveSettings round-trip continue to preserve \${VAR:default} literals on disk. New 'resolved' field carries the in-memory settings module run through the redactor — admin SPA can use it to show actual runtime values next to env-var placeholders. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Admin SPA now stores the resolved field from the /settings socket payload and exposes useResolvedAt(path) to walk it. EnvPill renders a "→ active value" chip when the path is resolved, or "→ ••••••" with a redacted tooltip when the server returned the [REDACTED] sentinel. Old-server fallback (undefined resolved) keeps current behaviour. The admin test script glob now picks up .test.tsx alongside .test.ts so the new EnvPill tests run under tsx --test. Closes #7803. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Review Summary by QodoShow resolved runtime values on /admin/settings with secret redaction
WalkthroughsDescription• Server emits resolved runtime settings alongside raw file blob - In-memory settings module with env vars substituted - Secrets redacted to [REDACTED] sentinel • Admin SPA stores resolved values and displays them in EnvPill widget - Shows → active value chip for resolved env-substituted settings - Shows → •••••• indicator for redacted secrets • Backwards compatible with old servers and clients - Old client ignores new resolved field - New client degrades gracefully when resolved is null • Comprehensive test coverage for redactor, path walker, and UI components Diagramflowchart LR
A["In-memory settings<br/>with env vars resolved"] -->|redactSettings| B["Redacted clone<br/>secrets → [REDACTED]"]
B -->|socket emit| C["Admin SPA<br/>resolved field"]
C -->|useResolvedAt| D["EnvPill widget"]
D -->|renders| E["→ active value chip<br/>or → •••••• redacted"]
F["Raw settings.json<br/>${VAR:default} intact"] -->|unchanged| G["Textarea + saveSettings<br/>preserve template"]
File Changes1. src/node/utils/AdminSettingsRedact.ts
|
Code Review by QodoLooking for bugs?Check back in a few minutes. An AI review agent is analyzing this pull request.
|
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
1 participant
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.
Summary
Closes #7803.
/admin/settingsno longer lies to operators when settings are env-substituted.adminsettings.tssocketloadhandler): emits a newresolvedfield alongside the existing raw-fileresultsblob. Theresolvedfield is the in-memorysettingsmodule — i.e. with${VAR:default}substitutions already applied at boot bylookupEnvironmentVariables— run through a small redactor that replaces known sensitive paths (users.*.password,dbSettings.password,sso.clients[*].client_secret,sessionKey, …) with the sentinel string[REDACTED].resultsandsaveSettingsare unchanged, so the raw textarea and round-trip save preserve${VAR:default}literals on disk. Container redeploys keep working untouched.resolvedpayload is cached in the store;useResolvedAt(path)walks it via the existing jsonc-parser JSONPath.EnvPill(the widget that already detects env placeholders and lets you edit the default) now renders a→ active valuechip when a resolved value is available, or→ ••••••with a tooltip when the server returned the[REDACTED]sentinel.resolved.resolvedisnull,useResolvedAtreturnsundefined, EnvPill omits the chip — degrades to today's behaviour.Reproducer (now fixed)
Before:
/admin/settingsshows the dbType pill's default asdirty(the template default).After: same pill now also shows
→ active value: sqlitereflecting what Etherpad is actually running.Test plan
resolved.trustProxy === trueandresolved.dbSettings.password === '[REDACTED]'. All 32 admin specs (existing + new) pass.tsx --test): 8resolveByPathunit tests + 5EnvPillrender tests covering undefined / resolved / redacted / non-string / null states. 21 admin tests pass overall (note:admin/package.jsontest glob updated to include*.test.tsxsince the only existing tests were*.test.ts).pnpm gen:api && tsc --noEmitclean onadmin/andsrc/.vite buildof admin SPA succeeds.docker run -e DB_TYPE=sqlite etherpad/etherpad, open /admin/settings, verify dbType pill shows→ sqliteand any secret-shaped setting shows the redacted indicator.Skipped: full Playwright e2e
I considered a Playwright test that boots Etherpad with
DB_TYPEset and asserts the chip text in a real browser. Skipping for now because the existing layers already cover every link in the chain (redactor → socket payload → store →useResolvedAt→EnvPillrender output), and the Playwright admin-auth setup overhead is large for a 5-line widget change. Happy to add one if reviewers want it.Allow-list for redaction
users.*.password,users.*.passwordHash,users.*.hash,dbSettings.password,dbSettings.user,sso.clients[*].client_secret,sso.clients[*].secret,sessionKey. Easy to extend insrc/node/utils/AdminSettingsRedact.tsif other plugins land paths that need protection.dbSettings.filenameis intentionally NOT redacted — operators need to verify their volume mounts.🤖 Generated with Claude Code