Skip to content

fix(server): preserve inline provider secrets on redacted saves - #10054

Merged
t3dotgg merged 2 commits into
pingdotgg:mainfrom
maxwellyoung:codex/preserve-inline-provider-secrets-20260905
Sep 6, 2026
Merged

fix(server): preserve inline provider secrets on redacted saves#10054
t3dotgg merged 2 commits into
pingdotgg:mainfrom
maxwellyoung:codex/preserve-inline-provider-secrets-20260905

Conversation

@maxwellyoung

@maxwellyoung maxwellyoung commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Saving an unchanged provider can erase a sensitive environment value stored inline in settings.json. Clients return a redacted placeholder, but there may not yet be a corresponding secret-store entry.

Before persisting that placeholder, migrate the current inline value for the same provider instance and variable into the secret store. Explicit replacements and clears keep their existing behavior.

Fixes #10022.

Validation: the regression fails on main and passes with the fix. All 38 server-settings tests pass, including duplicate names using the existing last-value-wins behavior, fresh-service reload, explicit replacement, clearing, and preserving the original file and cached value if the secret-store write fails. Targeted lint and server typechecking pass.

Note

Preserve inline provider secrets on redacted saves in serverSettings

  • Sensitive environment-variable updates with redacted values now look up the last existing non-empty inline value and migrate it to the secret store, instead of treating the redacted entry as empty.
  • Added tests covering secret migration failure (file unchanged, inline value retained) and parameterized cases for redacted saves, duplicate env-var names, explicit replacements, and explicit empty values.
  • Risk: if multiple existing entries share the same env-var name, only the last one is used; callers relying on the old empty-on-redacted behavior will see different persisted values.

Macroscope summarized 5b1c0f1.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 5, 2026
Comment thread apps/server/src/serverSettings.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The production change alters how redacted provider credentials are recovered from inline settings and written to the server secret store, affecting settings persistence and provider launch environments. Although narrowly scoped and covered by tests, its handling of credential material warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@Mnigos Mnigos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran this locally. All 38 server-settings tests pass, and with serverSettings.ts reverted to main exactly three of them fail (the redacted-save preserve case, the duplicate-name case and the secret-store failure case), so they really pin the fix. Also tried something I was curious about: this test file passes unchanged against #10053's implementation too, so the two PRs do the same thing and the difference is coverage. This one additionally locks in duplicate names, explicit replace and clear, and the write failure leaving settings.json untouched, which is the part I'd want covered for a bug that silently wipes credentials.

@t3dotgg
t3dotgg merged commit ac4f1a2 into pingdotgg:main Sep 6, 2026
24 checks passed
BarretoDiego pushed a commit to BarretoDiego/t3code that referenced this pull request Sep 7, 2026
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 7, 2026
## What's Changed
* fix(clients): show feedback results in composer banners by @juliusmarminge in pingdotgg/t3code#10398
* fix(server): stop Windows terminal polling from spiking CPU by @UtkarshUsername in pingdotgg/t3code#9476
* fix(web): onboarding installs agents without needing Node or npm by @t3dotgg in pingdotgg/t3code#10402
* fix(server): allow settling threads with unanswered async questions by @t3dotgg in pingdotgg/t3code#10400
* feat(ci): ship stable releases from the latest nightly commit by @t3dotgg in pingdotgg/t3code#10410
* feat(marketing): add a nightly channel to the download page by @t3dotgg in pingdotgg/t3code#10408
* fix(web): keep settings inputs focused during IME composition by @Lucenx9 in pingdotgg/t3code#10262
* fix(server): preserve Codex reset credits during usage updates by @yashranaway in pingdotgg/t3code#10308
* docs: link the repository security reporting policy by @yashranaway in pingdotgg/t3code#10303
* fix(web): only show auto balance errors after failed checks by @maria-rcks in pingdotgg/t3code#10407
* fix(web): improve preview recording frame delivery by @maria-rcks in pingdotgg/t3code#10403
* fix(server): preserve inline provider secrets on redacted saves by @maxwellyoung in pingdotgg/t3code#10054
* fix(web, mobile): replace Apple desktop machine labels by @extoci in pingdotgg/t3code#10396
* fix(web): hide browser when the right panel starts closing by @Neel2107 in pingdotgg/t3code#10385
* fix(web): keep settings section headings description-free by @maria-rcks in pingdotgg/t3code#10415
* fix(usage): read and redeem hub reset credits through CLIProxyAPI by @juliusmarminge in pingdotgg/t3code#10395
* fix(web): deduplicate expanded tool labels and keep errors expandable by @Yash-Singh1 in pingdotgg/t3code#10420
* fix(server): skip git status scans while the index is locked by @Gigioxx in pingdotgg/t3code#9845
* fix(mcp): allow text-only preview snapshots by @juliusmarminge in pingdotgg/t3code#10232
* fix(claude): name the expired login or usage limit instead of a generic API error by @vitalyiegorov in pingdotgg/t3code#10321
* feat(mobile): queue a message while its attachment is still uploading by @juliusmarminge in pingdotgg/t3code#10404
* feat(mobile): show when an existing thread has a message waiting in the outbox by @juliusmarminge in pingdotgg/t3code#10405
* fix(codex): accept misalignment policy errors on thread resume by @realbakari in pingdotgg/t3code#10373
* fix(server): skip disabled settlement lookups by @t3dotgg in pingdotgg/t3code#10424
* fix(server): run OpenCode CLI commands sequentially by @t3dotgg in pingdotgg/t3code#10427
* feat(web): name the drop action while dragging sidebar threads by @SunkenInTime in pingdotgg/t3code#10378
* perf(web): keep the sidebar responsive during bulk thread updates by @t3dotgg in pingdotgg/t3code#10413
* fix(web): onboarding wizard now supports light mode by @t3dotgg in pingdotgg/t3code#10432
* feat(threads): dismiss async questions without replying by @t3dotgg in pingdotgg/t3code#10431
* fix(web): stop collapsing the composer when it loses focus by @t3dotgg in pingdotgg/t3code#10437
* fix(server): keep interrupted threads resumable after restarts by @maria-rcks in pingdotgg/t3code#10421

## New Contributors
* @Neel2107 made their first contribution in pingdotgg/t3code#10385
* @realbakari made their first contribution in pingdotgg/t3code#10373

**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260906.1316...v0.0.39-nightly.20260907.1325

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260907.1325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Sensitive provider env value stored inline in settings.json is wiped on first save

3 participants