DEVX-1016: Sign Rootscribe outbound webhooks with HMAC-SHA256 and send timestamp and instance-id headers

[allenahner]

Summary

Outbound webhooks can now be verified by their receivers, and receivers can tell RootScribe installs apart.

• HMAC-SHA256 signing. When a webhook secret is configured, every delivery — fireRaw retries and the Settings/wizard "Test" send alike — carries x-rootscribe-timestamp (Unix seconds) and x-rootscribe-signature: t=<sec>,v1=<hex>, where v1 is the HMAC-SHA256 of ${t}.${body} over the exact JSON string handed to fetch. The timestamp is recomputed per attempt so a retry after the 5s/30s backoff still lands inside a receiver's tolerance window. Primitive: server/src/webhook/sign.ts → signWebhook(secret, timestampSec, body).
• Instance id. New AppConfig.instanceId (default null), minted as a UUID on first run by ensureInstanceId() (called at startup and by every delivery so the header is never absent even if settings.json was hand-edited). Sent as x-rootscribe-instance on every delivery, signed or not. POST /api/config accepts it as a trimmed, header-safe token (1..128 chars of [A-Za-z0-9._:-]) — undici's fetch throws on control chars, which would silently break every delivery.
• Unsigned warning. Without a secret, the server logs once per process (module-level latch) that deliveries are unsigned.
• Write-only secret (review round 1). GET/POST /api/config never return webhook.secret — the API has no auth and Docker binds 0.0.0.0, so a LAN client could otherwise forge HMAC-valid deliveries. Responses carry webhook.secretConfigured instead (mirrors the token redaction). On POST, webhook.secret is tri-state: omitted = keep the stored value, "" = clear, non-empty = replace — so unrelated saves can't wipe a stored secret.
• Settings UI. Webhook Outbound gains a Signing Secret input (write-only draft with a "configured" placeholder) + Generate / Clear buttons (32 CSPRNG bytes as 64 hex chars) and an editable Instance ID. A blank instance id is omitted (keeps the minted value) rather than sent (server would reject it).
• Setup wizard. WebhookStep gets the same secret input + Generate and a read-only instance-id display via the config query.
• Draft-aware Test (review round 1). POST /api/config/test-webhook accepts an optional secret; testWebhook(url, secret?) signs with it (undefined = stored, "" = unsigned). Settings and the wizard pass the field's current value so Test verifies the value about to be saved.
• Corrupt config is never overwritten (review round 1). ensureInstanceId() mints in-memory only when settings.json exists but failed to parse, so startup can't clobber a hand-recoverable file with defaults + a UUID.
• README. New "Webhook signing" section (header table + Node verification recipe with timingSafeEqual and a replay window; the recipe was exercised against real signWebhook output). Headers bullet updated. Payload example now includes end_time_ms, which the code has always sent.
• Release. 0.1.1 → 0.2.0 (minor: new functionality) across the same file set as the 0.1.1 hotfix bump; CHANGELOG [0.2.0] section added. The two "rootscribe/0.1.1" UA literals in post.ts are consolidated into one USER_AGENT constant.

Related Issues

• Resolves: DEVX-1016
• Epic: DEVX-1001 (Phase 3c — single-child collapse, so this PR targets main directly and carries the version bump)

Acceptance criteria → evidence

Scenario	Covered by
Signed delivery: x-rootscribe-signature with t and v1, v1 verifies against the body	server/tests/webhook/post.test.ts "signed delivery…" + "re-signs every retry attempt…" (recomputes the HMAC in the test, no mocks)
Instance header always present; no signature without a secret	post.test.ts "always sends x-rootscribe-instance…", "no secret configured…", "mints and persists an instance id on the fly…"
Test webhook signed the same way	post.test.ts "testWebhook — signing + instance headers"
Settings persists secret + instance id; subsequent deliveries use them	tests/e2e/settings.spec.ts (UI reload + GET /api/config), server/tests/routes/config.test.ts instanceId cases, Settings.test.tsx save-payload cases

Test Plan

• TDD workflow followed (RED-GREEN-REFACTOR) — every production change was preceded by a failing test shown in the transcript
• Unit tests passing — pnpm test 830/830 (repeated clean full-suite runs)
• Coverage thresholds held — statements 95.52 / branches 86.97 / functions 96.19 / lines 97.19 vs bars 95 / 86 / 95 / 96
• pnpm lint and pnpm typecheck clean
• Playwright e2e — 20/20 (CI=1 ROOTSCRIBE_E2E_PORT=45471, isolated from the live local instance on 44471)
• README verification recipe validated against real signatures (valid / tampered body / wrong secret / stale / missing header)
• Manual: set a secret in Settings, fire a test webhook at a receiver, verify with the README recipe

Notes for reviewers

• The secret is write-only through the API (see above); users copy it at generation time. settings.json stays 0600.
• The one-time "unsigned" warning lives in its own test file (post-unsigned-warning.test.ts) because the latch is per module instance and Vitest isolates modules per file.
• Pre-existing flakiness in server/tests/routes/recordings.test.ts (and once a 503 in config.test.ts) showed up under full-suite load and passes on re-run; flagged separately, not touched here.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Resolve malformed-config overwrite, protect webhook secrets from unauthenticated exposure, and ensure Test uses unsaved secrets.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds HMAC-SHA256 webhook signing, persistent instance IDs, UI configuration, tests, documentation, and the 0.2.0 release metadata.

Changes:

• Signs webhook deliveries and retries with timestamped HMAC headers.
• Adds instance ID generation, validation, persistence, and UI controls.
• Updates tests, documentation, changelog, and package versions.

File summaries

File	Summary
web/src/test-factories/appConfig.ts	Adds instance ID factory support.
web/src/routes/setup/WebhookStep.tsx	Adds wizard secret and instance ID UI.
web/src/routes/setup/WebhookStep.test.tsx	Tests wizard signing controls.
web/src/routes/Settings.tsx	Adds secret and instance ID settings.
web/src/routes/Settings.test.tsx	Tests settings persistence and generation.
web/src/lib/webhookSecret.ts	Generates secure webhook secrets.
web/src/lib/webhookSecret.test.ts	Tests secret generation.
web/package.json	Bumps web version.
tests/e2e/settings.spec.ts	Covers settings persistence end-to-end.
shared/src/config.ts	Extends shared configuration.
shared/src/config.test.ts	Tests configuration defaults.
shared/package.json	Bumps shared version.
server/tests/webhook/sign.test.ts	Tests HMAC signing.
server/tests/webhook/post.test.ts	Tests signed deliveries and retries.
server/tests/webhook/post-unsigned-warning.test.ts	Tests unsigned warning behavior.
server/tests/routes/config.test.ts	Tests instance ID validation.
server/src/webhook/sign.ts	Implements HMAC signing.
server/src/webhook/post.ts	Adds signed delivery headers.
server/src/test-seed/fixtures.ts	Adds deterministic seed instance ID data.
server/src/test-seed/fixtures.test.ts	Tests seed behavior.
server/src/routes/config.ts	Validates and persists instance IDs.
server/src/index.ts	Initializes instance IDs at startup.
server/src/config.ts	Generates and persists instance IDs.
server/src/config.test.ts	Tests instance ID generation.
server/package.json	Bumps server version.
README.md	Documents webhook signing and headers.
package.json	Bumps root version.
inbox-mcp/src/index.ts	Updates MCP version.
inbox-mcp/package.json	Bumps MCP package version.
CHANGELOG.md	Documents the 0.2.0 release.

Review details

Suppressed comments (1)

server/src/webhook/post.ts:43

• This helper is also used by testWebhook, but it always reads the persisted loadConfig().webhook.secret. The new Settings/wizard secret is only persisted after Save/Next, so clicking Test immediately after Generate or editing the field sends an unsigned request or uses the old secret while the UI displays the new one. Pass the pending secret to the test endpoint, or disable Test until the settings are saved.

  const secret = loadConfig().webhook?.secret;

• Files reviewed: 30/30 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Two critical unresolved configuration issues can break webhook delivery and overwrite stored webhook settings.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 33/33 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

A critical persisted-secret issue and three moderate review findings remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

server/src/routes/config.ts:124

• Now that secret is part of this schema, a request with a valid URL but a non-string secret also reaches this branch and is reported as invalid URL. Return a generic invalid-body message or inspect the validation path so API consumers are not misled about which field failed.

const TestWebhookSchema = z.object({
  url: z.string().url(),
  secret: z.string().optional(),
});

web/src/routes/Settings.tsx:307

• After a successful Test, changing the signing secret leaves the old result visible: the input, Generate, and Clear handlers never call setTestResult(null). The UI can therefore continue showing “Connection Success” for an unsigned/old-secret request after the draft has changed. Clear the test result whenever any secret mutation occurs.

                  setWebhookSecret(generateWebhookSecret());

web/src/routes/setup/WebhookStep.tsx:109

• After a successful Test Connection, editing or regenerating the draft secret leaves the old success/failure result visible because those handlers never clear testResult. That result may describe an unsigned or old-secret request, so it is misleading once the value used for the next test has changed. Clear the result whenever the secret changes, including Generate.

• Files reviewed: 33/33 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Address the retry-secret capture, instance ID validation, and setup-wizard secret handling findings before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

web/src/routes/setup/WebhookStep.tsx:107

• This placeholder promises that leaving the field blank sends unsigned, but that is false when the wizard is resumed after a partial setup with a stored secret: saveAndContinue omits secret and the route deliberately preserves the stored value, while Test also passes undefined and uses it. Because setup can be left before completeSetup, add an explicit clear path here or change the copy to say the existing secret is kept.

server/src/webhook/post.ts:172

• This re-reads the persisted secret for every retry. If an event gets a 503 and the operator rotates or clears the secret before the 5s/30s retry, the original URL receives that retry signed with a different key and rejects it, turning a transient failure into a permanent delivery failure. Capture the secret once for the delivery while still recomputing the timestamp/signature per attempt.

        headers: deliveryHeaders(event, body, loadConfig().webhook?.secret),

• Files reviewed: 33/33 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved persistence, secret-status, and setup-wizard data-loss issues must be fixed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

server/src/routes/config.ts:24

• usableSecret() treats a non-string persisted secret as unusable and sends deliveries unsigned, but this response flag marks any truthy value as configured. With a hand-edited webhook.secret: 12345 (the delivery path explicitly handles this case), Settings will show “configured” even though no signature is emitted, which can make the operator believe verification is active. Base secretConfigured on the same non-empty-string check.

    ? (({ secret, ...rest }) => ({ ...rest, secretConfigured: Boolean(secret) }))(cfg.webhook)

• Files reviewed: 33/33 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The setup wizard can clear stored webhook data while its config query is loading or has failed, and the config route has a lint error.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

server/src/routes/config.ts:27

• This destructured secret binding is intentionally discarded, but the repository's ESLint rule only ignores unused variables whose names start with _ (eslint.config.js:41-44). As written, pnpm lint reports secret as unused. Rename the binding while preserving the property omission (for example, destructure secret: _secret).

    ? (({ secret, ...rest }) => ({

shared/src/config.ts:19

• This comment misstates the contract: PatchSchema explicitly accepts webhook.secret and the route persists it, while secretConfigured is the response-only field that Zod drops from incoming patches. Keeping the comment as written can lead future changes to remove the write path or incorrectly treat the secret as non-persistent.

  // Response-only: whether a secret is stored. The server strips it from
  // incoming patches (Zod drops unknown keys) and never persists it.
  secretConfigured?: boolean;

• Files reviewed: 33/33 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Three moderate findings remain around stale settings saves and secret-only webhook Test invalidation.

Review details

Suppressed comments (3)

web/src/routes/Settings.tsx:105

• This save always includes the current webhook value, but the form can be hydrated from stale React Query data while /api/config is refetching (main.tsx:13). If the cache says webhook: null and the user edits only the instance ID or poll interval before the refetch completes, this posts webhook: null and clears the server's current URL and signing secret. Disable the form/save until the config fetch settles, or omit webhook when none of its fields were edited.

      await api.updateConfig({
        webhook: webhookUrl.trim()
          ? {
              url: webhookUrl.trim(),
              enabled: true,

web/src/routes/Settings.tsx:64

• The dirty guard prevents this hydration effect from running while any Settings field is being edited, and the invalidation check only compares URL and instance ID. If the stored signing secret changes during an in-flight Test (for example, another config update while the user is editing an unrelated field), a blank draft still means “use the stored secret,” but the old response is accepted and shown as verification of the new secret. Invalidate the test generation on every config replacement or otherwise track the redacted secret state/gate Test while it is being refetched.

    if (!cfg.data || dirty) return;
    const c = cfg.data.config;
    // Server data is replacing the draft. If anything a Test stamps (URL,
    // instance id) actually changes, an in-flight Test described the old
    // values and must not be rendered as success for the new ones.

web/src/routes/setup/WebhookStep.tsx:41

• A config refetch that changes only the persisted signing secret does not invalidate an in-flight Test: when the URL is untouched, stored === url skips invalidateTest(), and when the URL was typed, the earlier guard returns before this check. With a blank draft, the request uses the stored secret, so a response for the old key can still be rendered as success after rotation. Invalidate the test generation for any config replacement, not just a URL change.

• Files reviewed: 33/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

server/tests/webhook/post.test.ts:620

• This retry test validates each HMAC against that attempt's own timestamp, but never asserts that timestamps/signatures are refreshed between attempts. Since the suite leaves Date.now() real and only advances timers, all attempts can share one second; a regression that computed the headers once before the retry loop would still pass. Advance the clock per retry and assert the later timestamp/signature changes.

    for (const call of fetchMock.mock.calls) {
      const init = call[1] as RequestInit;
      const headers = init.headers as Record<string, string>;
      const { t, v1 } = parseSignature(headers["x-rootscribe-signature"]!);
      expect(v1).toBe(expectedSignature(secret, t, String(init.body)));

• Files reviewed: 33/33 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Settings Save and Test can use stale cached webhook configuration after a failed refetch.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

web/src/routes/Settings.tsx:133

• When the config query has cached data, React Query can keep that data while a refetch is in flight or after it fails. This save path is still enabled and, after only the secret is edited, combines the draft secret with the cached webhook URL; if another client removed or changed the webhook, this can resurrect or rewire it. Gate Settings Save (and Test) on a successful, non-fetching config query or otherwise require a fresh/re-entered URL, and cover the cached-refetch-error case.

This issue also appears on line 309 of the same file.

web/src/routes/Settings.tsx:309

• This Test button remains enabled while the shared config query is refetching or has errored with cached data. In that window test() passes the locally hydrated instanceId (and an omitted draft secret is resolved against the server), so a stale instance id can be tested even though a subsequent Save would omit it and use the current server value; a fast response can render a misleading success before dataUpdatedAt invalidates it. Gate Test on a successful, non-fetching config query as WebhookStep does.

              disabled={!webhookUrl || saving}

• Files reviewed: 33/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Two moderate findings (one vote each) remain because secret-only edits in Settings and setup can re-enable disabled webhooks.

Review details

Suppressed comments (2)

web/src/routes/Settings.tsx:144

• A secret-only edit can target a webhook that is currently disabled, but this payload unconditionally writes enabled: true. Rotating or clearing the signing secret would therefore start deliveries unexpectedly for a stored { url, enabled: false } config; preserve the stored enabled value when webhookUrl was not touched and only enable when the URL is explicitly configured/changed.

              webhook: webhookUrl.trim()
                ? {
                    url: webhookUrl.trim(),
                    enabled: true,
                    // Tri-state on the wire: omitted = keep stored, "" =

web/src/routes/setup/WebhookStep.tsx:125

• When this step is revisited with a stored webhook whose enabled flag is false, clicking Next (for example after only entering a secret) rewrites it as enabled: true and starts deliveries without an explicit URL change. Preserve the stored enabled state for an untouched hydrated URL, while keeping true for a newly entered URL.

• Files reviewed: 33/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Three unresolved moderate issues affect stale configuration state and test-result invalidation during refetches.

Review details

Suppressed comments (3)

web/src/routes/Settings.tsx:162

• If the POST succeeds but the follow-up invalidateQueries refetch fails, React Query keeps the old cached config and marks the query errored; this code still clears touched. The hydration effect then rehydrates the form from that stale cache, so the newly saved URL/secret/instance ID disappears from the UI and secretConfigured can incorrectly revert until a reload. Only clear touched after confirming the config query has a successful refreshed state, or seed the query cache from the successful POST response before clearing the drafts.

      await qc.invalidateQueries({ queryKey: ["config"] });
      setTouched(NOTHING_TOUCHED);

web/src/routes/Settings.tsx:106

• dataUpdatedAt changes only when a refetch returns new data. If a config refetch is still in flight or finishes with an error, this effect never invalidates the generation, so an already-running Test can resolve afterward and display success for cached/unknown secret and instance values even though configSettled is false. Invalidate when cfg.isFetching starts (or on every refetch outcome) so no result survives a config refresh.

  useEffect(() => {
    if (cfg.dataUpdatedAt) invalidateTest();
    // invalidateTest is stable in effect (bumps a ref, clears state).
  }, [cfg.dataUpdatedAt]);

web/src/routes/setup/WebhookStep.tsx:45

• dataUpdatedAt changes only when a refetch returns new data. If a config refetch is still in flight or finishes with an error, this effect never invalidates the generation, so an already-running Test Connection can resolve afterward and display success for cached/unknown secret and instance values even though the button is gated by cfg.isFetching/cfg.isError. Invalidate when cfg.isFetching starts (or on every refetch outcome) so no result survives a config refresh.

• Files reviewed: 33/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Resolve the two moderate webhook/config findings and the shared response-type mismatch before approval.

Review details

Suppressed comments (3)

server/src/routes/config.ts:44

• redactForClient maps an omitted enabled to false, but the POST normalizer treats an omitted flag as url.length > 0. A persisted/hand-edited { url: "https://..." } therefore appears disabled to Settings, and a secret-only save echoes that false value and leaves the webhook disabled instead of applying the documented default. Preserve an explicit boolean and use the same URL-present default when it is missing.

          enabled: Boolean(enabled),

server/src/webhook/post.ts:180

• The retry loop captures the signing secret once, but it does not capture the instance id: deliveryHeaders() calls ensureInstanceId() again on every attempt. If Settings changes the instance id during the 5s/30s backoff, the same logical delivery is sent first with the old identity and then with the new one, so receivers that use (instance,event) for attribution or deduplication can treat the retry as coming from a different install. Capture ensureInstanceId() once per fireRaw delivery and pass that value to every attempt, just like the secret.

  const secret = loadConfig().webhook?.secret;
  for (let attempt = 0; attempt < BACKOFF_MS.length; attempt++) {
    const started = Date.now();
    try {
      const res = await fetch(url, {
        method: "POST",
        headers: deliveryHeaders(event, body, secret),

shared/src/config.ts:21

• ConfigResponse still aliases AppConfig, so adding secret?: string here makes the shared response type advertise that GET/POST config may contain the secret even though redactForClient() deliberately strips it. A future TypeScript client can legally read config.webhook.secret and silently get undefined, undermining the write-only contract. Split the persisted/patch webhook shape from the redacted response shape (or otherwise omit secret from ConfigResponse) while retaining it in the POST patch type.

  secret?: string;
  // Response-only flag: true when a non-empty string secret is stored. The
  // secret itself IS accepted and persisted on POST (see above); this flag
  // is the read side. PatchSchema does not list `secretConfigured`, so Zod
  // drops it if a client echoes it back — it is never persisted.
  secretConfigured?: boolean;

• Files reviewed: 33/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved moderate findings affect webhook configuration consistency, stale URL preservation, and wizard cache state.

Review details

Suppressed comments (4)

server/src/routes/config.ts:44

• This coercion turns a missing or invalid persisted enabled value into false, while the POST normalizer below treats an omitted flag as url.length > 0. For a hand-edited but otherwise usable webhook such as { url, secret }, the API therefore reports it disabled; editing only the secret/instance in Settings or revisiting the wizard echoes enabled: false and can stop deliveries. Preserve boolean values and apply the same URL-present default for missing/invalid values.

          enabled: Boolean(enabled),

web/src/routes/Settings.tsx:158

• A secret-only save still sends the hydrated URL and enabled inside webhook. If another Settings tab/client changes the webhook URL after this page's last config fetch (before React Query starts a refetch), this request overwrites that newer URL with the stale one; the per-field tracking cannot prevent it because webhook is present in the patch. Add optimistic version/conflict handling or a server-side secret-only patch that preserves the current URL.
  web/src/routes/setup/WebhookStep.tsx:139
• This invalidation happens after a successful save, but unlike Settings it does not seed ['config'] with the POST response. If the follow-up GET fails, React Query retains the pre-save cached config (for example webhook: null), so navigating back to this step shows stale URL/secret state despite the server having saved the draft (and may leave the user at Skip/unable to test). Capture the updateConfig response and call setQueryData before invalidating, as Settings does.
  web/src/routes/setup/WebhookStep.tsx:134
• When a resumed wizard only adds/rotates a secret, this request still includes the hydrated URL and enabled flag. If another client changes that URL after the wizard's last config fetch, the save overwrites the newer URL with stale state; urlTouched only controls the enabled default, not whether the stale URL is sent. Use a conflict/version check or a patch that updates the secret without replaying the URL.

• Files reviewed: 34/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

server/src/routes/config.ts currently coerces webhook.enabled with Boolean(...), which can mis-report a hand-edited non-boolean value (e.g. "false") as enabled and should be sanitized more strictly.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

server/src/routes/config.ts:47

• redactForClient() coerces enabled with Boolean(enabled), which will treat any truthy non-boolean (e.g. a hand-edited string "false") as enabled. Since this endpoint is explicitly defending against hand-edited/invalid persisted shapes, it’s safer to accept only a real boolean and default everything else to false (so the UI doesn’t show an enabled webhook that may not be intentionally enabled).

• Files reviewed: 34/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Copilot review overview

🔵 Needs a closer look

The broad cross-layer security, persistence, UI, and release changes warrant final human review.

Review effort: Lite
Findings: None