From 8f14d371ad5e4294db3a0174585277581d91fad1 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 22 Aug 2026 22:30:21 -0400 Subject: [PATCH] fix(auth): land the round-20 UI wiring that never applied, and nonce the staging name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of round 21's four findings say the same thing: **the UI half of round 20 was never applied, and I reported it as done.** The edit script aborted partway on a regex error, after writing one file and before writing the other two. So `ServerConfigModal` gained the footer prop but `App.tsx` never passed it — the footer was `undefined` in production and rendered nothing, visible only to the unit test that passes a descriptor directly — the extra refresh call sites never landed, and the footer's doc comment still claimed two secret-entry dialogs while the code had three. `npm run ci` passed throughout, because every test I added exercises the piece that *did* land. This is the same failure as round 17's uncommitted files with a different cause: I verified intent (green gate, clean typecheck) instead of the artifact. Each edit now asserts it changed the file, and the result is grepped afterwards — the check costs a second and is the only one that describes what is actually on disk. Landed now, and verified in the files rather than inferred: - `App.tsx` passes `secretStorage` to `ServerConfigModal`, so the disclosure the previous commit claimed to add actually renders. - Every secret-bearing write refreshes the descriptor — config submit and both import flows, not just the two settings saves. Any of them can perform the pending plaintext-to-encrypted upgrade, after which the footer was reporting "Plaintext file" until reload. - The footer's doc names all three dialogs. The wrong count there was not a description of the bug, it *was* the bug: the omitted dialog was the one taking secrets with no disclosure. Also, the fourth finding: **the staging name was reusable across restarts.** `recoverOrphanedSnapshots` deliberately leaves an orphan when a live `secrets.json` also exists, and a pid-only name repeats (pid 1 on every container start) — so the next claim would `rename` straight over that orphan and permanently discard secrets it may uniquely hold. A per-attempt nonce makes a staging path unrepeatable. Full `npm run ci` green, staged tree verified first. Signed-off-by: cliffhall --- clients/web/src/App.tsx | 18 ++++++++++++++---- .../SecretStorageFooter.tsx | 10 +++++++--- core/auth/node/secret-store-selection.ts | 8 +++++++- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 9e764b6b..e1685093 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -4591,21 +4591,31 @@ function App() { initialConfig={configModalTarget?.config} existingIds={existingIds} onClose={() => setConfigModal(null)} - onSubmit={onConfigSubmit} + // Wrapped like the settings persists: this submit carries stdio `env` + // values, so it can perform the pending plaintext-to-encrypted + // upgrade and change the descriptor the footer reports. + onSubmit={refreshingPersist(onConfigSubmit, refreshInitialConfig)} + secretStorage={secretStorage} /> setImportConfigOpen(false)} onFetchSource={importSource} - onAddServer={addServerHighlighted} - onUpdateServer={updateServer} + onAddServer={refreshingPersist( + addServerHighlighted, + refreshInitialConfig, + )} + onUpdateServer={refreshingPersist(updateServer, refreshInitialConfig)} /> setImportJsonOpen(false)} - onAddServer={addServerHighlighted} + onAddServer={refreshingPersist( + addServerHighlighted, + refreshInitialConfig, + )} />