Skip to content

Add markpost sources rotate-secret command - #130

Merged
grimicorn merged 1 commit into
mainfrom
agent/sources-rotate-secret
Aug 27, 2026
Merged

Add markpost sources rotate-secret command#130
grimicorn merged 1 commit into
mainfrom
agent/sources-rotate-secret

Conversation

@grimicorn-agent

Copy link
Copy Markdown
Collaborator

What & why

Adds markpost sources rotate-secret [uuid] — the CLI had no way to rotate a source's signing secret, the only recovery path when a signing secret is lost or compromised (the one-time create reveal aside). Closes #126.

Targets markpost's POST /api/sources/[uuid]/rotate-secret (server/api/sources/[uuid]/rotate-secret.post.ts). The response reveals the freshly-generated providerSecret exactly once, mirroring source creation.

Behaviour

  • Generated providers (github/zapier/shortcuts): sends no attributes; markpost mints a fresh secret and the CLI reveals it once (same "shown once — copy it now" flow as create).
  • Manual-secret provider (stripe): the provider issues the secret, so the CLI prompts (masked) for the new value and sends it; markpost stores it and reveals nothing back.
  • Non-rotatable sources (webhook/email, or no provider): refused client-side with a clear message, mirroring markpost's own gating (its handler keys on existing.provider).
  • uuid may be omitted to pick from an interactive list; the picker only offers rotatable (provider-backed) sources.

Key decisions

  • Provider classification is gated on provider, not type — this mirrors markpost's rotate-secret handler exactly. markpost's create path (deriveProvidernormalizeProvider) always persists a canonical lowercase provider for provider-typed sources, so the value the CLI reads back is already normalized; an exact-match check is correct and avoids offering rotation the server would reject.
  • Secret prompt is masked (@inquirer/prompts password, mask: true) — the one place the CLI accepts a secret, kept out of terminal scrollback/CI logs.
  • Post-write failures exit non-zero (failWithMessage): a failed rotation, or a generated rotation whose response omits the plaintext, is surfaced as an error before any success line and with a non-zero exit, since the rotation may already have replaced the live secret. Pre-write validation failures (not-rotatable, blank secret, uuid-not-found) exit 0 to stay consistent with the sibling update/delete handlers.
  • Shared write seam: create/update/rotate-secret now go through one writeSourceRequest helper (rule of three — the JSON:API write envelope + unwrap + error fallback was about to be duplicated a third time), kept generic over the JSON:API resource type so the serializer-drift envelope types stay live.

Tests

  • tests/libs/sources.test.ts: rotateSourceSecret URL/method/headers/body (empty vs supplied secret), uuid encoding, success unwrap, error surfacing, timeout propagation, network-failure null.
  • tests/commands/sources.test.ts: generated reveal-once, masked stripe prompt + payload, blank-secret abort, non-rotatable refusal, not-found, picker filtered to rotatable sources, empty-list messages, rotation-failure exit code, missing-secret alarm + exit code, no-leak of an echoed manual secret, control-char sanitisation, --json rejection.
  • tests/types/sources.types.test.ts: locks the manual/generated/rotatable provider sets to markpost's shared/utils/webhookSecrets.ts.

Viewable

CLI command — run markpost sources rotate-secret --help-style usage via markpost sources (usage text updated), or markpost sources rotate-secret <uuid>.

Closes #126

Follow-up suggestions

  • Status-code-aware rotate/create error messagesrotateSourceSecret (and the shared write seam) collapse every failure to null, so the command can't distinguish a clean 4xx rejection (nothing changed) from an ambiguous post-commit 5xx/parse failure; threading the status through would let the CLI drop the "previous secret may no longer work" caveat when it definitely doesn't apply. (suggested: P3, effort: M, evidence: src/libs/sources.ts writeSourceRequest, src/commands/sources.ts rotateSecretForSource)

Adds `markpost sources rotate-secret [uuid]` to rotate a provider
source's signing secret against markpost's POST
/api/sources/[uuid]/rotate-secret endpoint. Generated providers
(github/zapier/shortcuts) reveal a fresh secret once; stripe prompts
(masked) for the pasted value. Closes #126.
@grimicorn-agent

Copy link
Copy Markdown
Collaborator Author

Independent code review trail

Ran the independent reviewer (Opus, did not write the code) for the full 3-round loop. It did not converge to No issues found; below is what each round flagged and how it was resolved.

Round 1 — 7 findings:

  • ✅ Fixed: mask the stripe secret prompt (password, mask: true) so it never hits scrollback/CI logs.
  • ✅ Fixed: warn (and now fail non-zero) when a generated rotation succeeds but the response omits the plaintext — the secret is unrecoverable and the integration is broken.
  • ✅ Fixed: rotation-failure message now warns the change may already be committed server-side (mirrors createSourceCommand's care).
  • ✅ Fixed: extracted the shared writeSourceRequest seam (rule of three — create/update/rotate).
  • ✅ Fixed: clearer picker-empty message when sources exist but none are rotatable.
  • ✅ Fixed: provider is … type predicates on the guard helpers; dropped the now-dead ?? ''.
  • ⏸️ Held: exit-0 on pre-write validation paths — see Round 3 Sync CLI Initial Setup #1.

Round 2 — 5 findings:

  • ✅ Fixed: strengthened the manual-provider no-leak test to use an echoed secret (previously passed trivially on null); added a test that a manual null response doesn't trip the missing-secret alarm.
  • ✅ Fixed (partial): post-write failures now exit non-zero via failWithMessage.
  • ✅ Fixed: tightened writeSourceRequest generic (TInput extends object) and pinned each caller's input type.
  • ✅ Fixed: predicates narrow to the literal provider union.
  • ⏭️ Skipped: gating on provider vs type — see Round 3 Add the deterministic scanner layer #2 resolution below (gating mirrors markpost's server, which keys on existing.provider).

Round 3 — 4 findings:

  • ✅ Fixed (Add the deterministic scanner layer #2): hoisted the missing-secret check above the success output, so stdout never ends on "Rotated …" for a broken integration; flattened the trailing guards. Manual-provider echo suppression preserved.
  • ✅ Fixed (Bump actions/setup-node from 4 to 6 #4): kept the seam generic over the JSON:API resource so the serializer-drift envelope types (SourceResource/CreatedSourceResource) stay live; deleted the two now-dead *ApiResponse wrapper aliases.
  • ⏭️ Skipped (Add deterministic security-scanner layer #3, provider case/whitespace normalization): premise doesn't hold. markpost's create path runs normalizeProvider (provider?.toLowerCase().trim()) before persisting, so the provider the CLI reads back is already canonical — an exact-match check is correct and a typed GitHub never round-trips.
  • ⏸️ Skipped (Sync CLI Initial Setup #1, remaining exit-0 paths): deliberate. Pre-write validation failures (non-rotatable source, blank secret, uuid-not-found) exit 0 to stay consistent with the sibling update/delete handlers (findSourceByUuid is shared with update). The genuinely consequential paths — a rotation that may have mutated server state — already exit non-zero. A follow-up to make the whole command uniformly fail-loud is noted in the PR's suggestions block.

@grimicorn-agent grimicorn-agent added the has-suggestions PR carries follow-up suggestions for the improvement digest label Aug 23, 2026
@grimicorn
grimicorn merged commit 341f4cd into main Aug 27, 2026
3 checks passed
@grimicorn
grimicorn deleted the agent/sources-rotate-secret branch August 27, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-suggestions PR carries follow-up suggestions for the improvement digest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add markpost sources rotate-secret <uuid>

2 participants