fix(messaging): dashboard broadcast never leaks PIN placeholder to user#942
Open
JKHeadley wants to merge 1 commit into
Open
fix(messaging): dashboard broadcast never leaks PIN placeholder to user#942JKHeadley wants to merge 1 commit into
JKHeadley wants to merge 1 commit into
Conversation
broadcastDashboardUrl printed the literal "(check your config)" in place of
the dashboard PIN whenever this.config.dashboardPin was falsy. Live incident
(2026-06-06, topic 5): a transient vault/keychain read failure during a boot
under host load left the loadConfig secret-merge incomplete, so dashboardPin
arrived as the unresolved {secret:true} ref and the placeholder leaked to the
user as their PIN — a value they cannot act on, reading like an instruction.
Fix mirrors the constructor's existing {secret:true} normalization for
config.token (v1.3.270 boot-crash incident), extended to the PIN:
- new src/core/dashboardPin.ts — pickDashboardPin / resolveDashboardPinFromVault
(pure, never-throws vault read; in-memory-first, vault fallback; never the
placeholder). Mirrors ghToken.ts.
- TelegramAdapter.resolvedDashboardPin() seam re-resolves the PIN at send time,
recovering the real PIN from a transient boot failure.
- formatDashboardMessage accepts a nullable PIN; a null PIN omits the line with
an honest, actionable note instead of ever emitting the placeholder.
25 unit tests (dashboard-pin-vault 16 + telegram-dashboard-pin-leak 9);
canaries 81/81; tsc + full lint clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
ELI16
When your agent sets up a phone-friendly web dashboard, it posts a small pinned message with the link and a 6-digit PIN you type to log in. That PIN lives in the agent's encrypted vault and gets unlocked when the server starts. If the server happened to restart while the machine was under heavy load, that unlock could briefly fail — and when it did, the message printed the words "(check your config)" where your PIN should have been. That's leftover internal text you can't act on, and the dashboard link came with no usable PIN.
This fixes it. When the broadcast goes to post your link, it uses the PIN it already has; if that's missing, it reaches back into the vault and reads your real PIN right then (recovering from the startup hiccup). And if it truly can't find your PIN in that moment, it now just leaves the PIN line out with an honest "ask me for your dashboard PIN and I'll send it" — instead of ever pasting placeholder text. Your dashboard message will always show your real PIN or a plain "just ask me" — never confusing wording in place of the code you need.
What Changed
TelegramAdapter.broadcastDashboardUrlprinted the literal(check your config)in place of the dashboard PIN wheneverthis.config.dashboardPinwas falsy. Live incident (2026-06-06, topic 5): a transient vault/keychain read failure during a server boot under host load left theloadConfigsecret-merge incomplete, sodashboardPinarrived as the unresolved{secret:true}ref and the placeholder leaked to the user as their PIN.The fix mirrors the constructor's existing
{secret:true}normalization forconfig.token(added v1.3.270 after the boot-crash incident — the same unresolved-secret failure mode), extended to the PIN:src/core/dashboardPin.ts(new) —pickDashboardPin(inMemory, stateDir)/resolveDashboardPinFromVault(stateDir): pure, never-throws vault read; in-memory value first, then a fresh vault read; never returns the placeholder. MirrorsghToken.ts.TelegramAdapter.resolvedDashboardPin()(new private seam) — re-resolves the PIN at send time, recovering the real PIN from a transient boot-time failure. The seam makes the broadcast wiring unit-testable without network I/O.formatDashboardMessage(url, pin: string | null, isNamed)— accepts a nullable PIN; anullPIN omits the PIN line with an honest, actionable note instead of ever emitting the placeholder. The placeholder string is removed from the adapter entirely.CoherenceMonitor's(check your config)output-sanity pattern is left in place as a defense-in-depth backstop.Evidence
tests/unit/dashboard-pin-vault.test.ts(16) — resolver + picker against a REAL on-disk SecretStore (forceFileKey): happy path, trimming, absent vault, wrong key, empty/whitespace, the unresolved{secret:true}object, placeholder-never-returned, corrupt-vault never-throws, production dual-key read, in-memory-first preference, vault fallback for every unresolvable in-memory shape.tests/unit/telegram-dashboard-pin-leak.test.ts(9) — the adapter: renders a real PIN (named + quick), omits the line with an honest note on null and NEVER emits the placeholder, and end-to-end recovers the real PIN from the vault when config holds the unresolved object (never[object Object]).tsc --noEmitclean; fullnpm run lintclean.Testing-tier applicability
A fix to formatting/resolution logic inside an existing messaging method — no new HTTP route or server-lifecycle surface, so Tier 2/3 have no applicable surface. Tier 1 covers it fully, including the wiring-integrity seam (
resolvedDashboardPin) the Testing Integrity Standard requires.Migration parity
Core source compiled into the shipped package — reaches all agents via the normal package/server update. No agent-installed file (hook/config/CLAUDE.md/skill) touched, so no
PostUpdateMigratorentry is required.🤖 Generated with Claude Code