Repository navigation
Description
Impact
The shared secret that authenticates ObjectStack to a webhook receiver is stored verbatim, per delivery attempt, in a table readable over the ordinary data API. Anyone who can read sys_http_delivery recovers the signing key for every subscriber and can mint payloads the receiver accepts as genuine — the signature is the receiver's only proof of origin.
Admin-gated on this deployment, so this is defense-in-depth rather than privilege escalation. But the blast radius is external: it compromises the trust boundary at systems ObjectStack does not control, and rotating a leaked key means re-coordinating with every receiver operator.
Root cause (located)
sys_http_delivery carries the secret as a plain column alongside headers_json; the outbox writer persists it with the attempt rather than resolving it at send time.
The platform already has the machinery to avoid this and uses it elsewhere: sys_setting stores a sec_ handle with the ciphertext in sys_secret (aes-256-gcm). And the same QA run proved the sibling datasource path gets it right — an inline secret on POST /api/v1/datasources is split out server-side, and a byte-level scan of every table and column of the running DB found no occurrence of it.
So this is an inconsistency between two sibling subsystems, not a missing capability — which is what makes it tractable.
Reproduction
- Author a webhook subscriber with a signing secret and trigger a delivery.
- Read the delivery row over the ordinary data API as an admin:
GET /api/v1/data/sys_http_delivery. - The authored secret comes back verbatim on the attempt row.
Found as an adjacent observation while verifying HMAC correctness: the runner recomputed HMAC-SHA256(raw body, <secret>) against the delivered x-objectstack-signature — exact match, so signing itself is correct — and then noticed the secret was readable from the delivery table.
Acceptance criteria
- The signing secret is not recoverable from
sys_http_delivery(or any delivery/outbox row) over the data API, for any persona. - Signing still works end-to-end: a delivered payload's
x-objectstack-signaturestill verifies against the subscriber's secret, asserted by recomputing the HMAC over the raw body — not by asserting "no error". - A regression guard reads the delivery table directly after a real delivery and asserts the secret's bytes are absent (the byte-level-scan shape the datasource path already passes).
Suggested shape (not prescriptive)
Store a handle rather than the value on the delivery row and resolve it at send time, matching the sys_setting / sys_secret split. Failing that, at minimum keep it off the delivery record entirely — it belongs to the subscriber, not to each attempt — and exclude the column from the data-API projection.
Worth checking in the same pass whether any other outbox/delivery table copies a credential the same way.
Note on target:<major>
Not applied here — that label has a single producer and the maintainer has not ruled on this card. Flagging the tension for that ruling rather than pre-empting it: this is admin-gated (the #7522 precedent for an admin-gated cleartext read was not boarded), but unlike #7522 its blast radius reaches outside the deployment, which is an argument for boarding it. Maintainer's call.
Provenance & disclosure
Held back from the public run card #7690 pending disclosure handling (D1 precedent); the private write-up is docs/qa/platform-checklist/FOLLOW-UPS.md §1a entry D12 (branch claude/platform-test-checklist-ocwugl @ 1ce13eb). Filed publicly under the maintainer's standing instruction of 2026-08-11 (verbatim): 「有问题就立卡片」.
Source
QA run #7690 (webhook-lifecycle, adjacent observation; framework 92f26f75), private write-up FOLLOW-UPS.md §1a D12. The webhook item itself passes — signature computation, custom headers, per-attempt durability, timeout enforcement and the retry ladder were all proven correct; this finding does not change that verdict.
Activity
huangyiirene commentedon Aug 11, 2026
Claim: PM loop round 1
Session:
session_01JVfWSQN9RDDGwdVSmMc29xBranch:
claude/issue-7722-webhook-secret-cleartextWorktree: cloud session's own clone (mode:cloud)
Domain:
domain:servicesFile surface:
packages/plugins/plugin-webhooks/src/**(auto-enqueuer.ts carriessigningSecretonto the enqueue at ~:387/:483; webhook-outbox-plugin.ts ownssys_http_delivery; sys-webhook.object.ts / schema.ts for the column/projection),packages/services/service-messaging/src/http-dispatcher.ts(send-time HMAC). Read-only consumption of the existingsys_secret/sec_handle machinery expected; if the fix requires changing the secret store itself, stop on breach and report.Container & model: M (security, cross-file redesign of where the secret lives),
mode:cloud,model: opusSerial constraints cleared: none — lane in-flight was 0 at claim; no open PR touches plugin-webhooks or service-messaging (same-day churn on both paths: version-packages rc only).
Generated by Claude Code
huangyiirene commentedon Aug 11, 2026
{ "issue": 7722, "premise_still_valid": true, "premise_evidence": "origin/main@7a8476f: packages/services/service-messaging/src/objects/http-delivery.object.ts:126 declared `signing_secret: Field.text({ label: 'HMAC Secret', maxLength: 256 })`; sql-http-outbox.ts:109 wrote `signing_secret: input.signingSecret` verbatim on every enqueue; the object declares `enable.apiMethods: ['get','list']`, so the column is readable over the ordinary data API. Both producers fed it cleartext — plugin-webhooks/src/auto-enqueuer.ts:387 and :483 (`signingSecret: sub.secret`) and service-automation/src/builtin/http-nodes.ts:133 (Flow `http` node).", "status": "draft_pr_open", "branch": "claude/issue-7722-webhook-secret-cleartext", "pr": 7798, "pr_url": "https://github.com/objectstack-ai/objectstack/pull/7798", "commit": "500944c", "shape_chosen": "Delivery rows carry the SIGNATURE, never the key. The body is fixed at enqueue and replayed byte-for-byte by retries and redeliver(), so the HMAC has exactly one correct value for the row's life: the outbox computes it once at enqueue from EnqueueHttpInput.signingSecret and persists only `signature` (`sha256=<hex>`), the same value the receiver is handed on the wire and one-way in the key. The secret is consumed and dropped.", "shape_rejected": { "handle_resolved_at_send_time": "The issue's first suggestion (sys_setting/sys_secret split, datasource-binder shape). Rejected on evidence: the datasource binder exists because a live connection pool must recover CLEARTEXT at runtime; a delivery never needs the key back, only one HMAC over one fixed body. A handle would still have minted a sys_secret row per delivery attempt — per-attempt ciphertext on a telemetry table with 30d retention and no reaper for sys_secret — plus a decrypt per retry, to reach the same end state. It also could not be wired without a composition-root change (packages/cli/serve.ts owns crypto-provider injection), which is outside the declared surface." }, "surface_touched": { "declared": ["packages/plugins/plugin-webhooks", "packages/services/service-messaging"], "outside_declared": ["content/docs/automation/webhooks.mdx", ".changeset/webhook-delivery-signature-not-secret.md"], "outside_declared_justification": "The mdx documents the renamed column (§3.2 table) and the signing model (§6) — leaving it would ship a doc naming a column that no longer exists. The changeset is a required local gate. No change to the secret store, packages/spec, packages/objectql, or any producer package.", "files": [ "packages/services/service-messaging/src/http-sender.ts", "packages/services/service-messaging/src/http-outbox.ts", "packages/services/service-messaging/src/sql-http-outbox.ts", "packages/services/service-messaging/src/memory-http-outbox.ts", "packages/services/service-messaging/src/objects/http-delivery.object.ts", "packages/services/service-messaging/src/index.ts", "packages/services/service-messaging/src/translations/{en,es-ES,ja-JP,zh-CN}.objects.generated.ts", "packages/services/service-messaging/package.json (devDeps: objectql, driver-sql — for the at-rest integration test)", "packages/services/service-messaging/src/http-outbox.test.ts", "packages/services/service-messaging/src/http-signature-at-rest.integration.test.ts (new)", "packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts (new)", "content/docs/automation/webhooks.mdx", ".changeset/webhook-delivery-signature-not-secret.md" ], "no_change_needed": "packages/plugins/plugin-webhooks/src/auto-enqueuer.ts — it hands `signingSecret` to the outbox, which is still the correct producer contract; what changed is that the outbox consumes rather than copies it." }, "acceptance_evidence": { "byte_scan_after_real_delivery": "http-signature-at-rest.integration.test.ts — real ObjectQL + @objectstack/driver-sql on better-sqlite3 :memory:, real DDL via syncSchemas(), production SqlHttpOutbox + HttpDispatcher. After a real delivery it runs `SELECT *` over every column of every delivery row (not a projection of declared fields, so a stale physical column could not hide) and asserts the secret's bytes are absent. The scan is itself guarded — it must also see the URL and the signature — so an empty dump can never be what makes it pass. Cases: webhook, flow, redelivery, unsigned.", "hmac_recomputed_over_raw_body": "Same file plus plugin-webhooks/src/webhook-signing-secret.test.ts (full chain from a data.record.created event through the real AutoEnqueuer). Each asserts `X-Objectstack-Signature === 'sha256=' + HMAC-SHA256(raw body received, secret)`, and that the signed body is the real event rather than an empty one an HMAC would also match. Also upgraded the pre-existing http-outbox.test.ts case from `toMatch(/^sha256=/)` to the same recompute.", "round_trip_pin": "The signature is computed BEFORE the row is written and the body is rebuilt AFTER a full stringify -> text column -> parse -> stringify round-trip, so the recompute assertion doubles as the guard that a changed byte breaks verification instead of shipping an unverifiable delivery. Payload fixture carries nesting, unicode, a float, an empty string.", "reverse_verification": "Reinstating the pre-fix column + `signing_secret: input.signingSecret` write fails 3 of the 4 at-rest cases, each on the byte-scan assertion (`AssertionError: expected '…' not to contain 'whsec_7722_do_not_persist_me'`). The 4th (unsigned delivery) legitimately passes either way. Temporary edits reverted before commit; `git grep signing_secret` over src is clean apart from one explanatory comment." }, "gates": { "service_messaging_suite": "pass — 18 files / 206 tests", "plugin_webhooks_suite": "pass — 4 files / 35 tests", "typecheck_dependents_closure": "pass — turbo typecheck --filter=...@objectstack/service-messaging, 77 tasks", "build_closure": "pass — turbo build over the dependents closure of both packages, 66 tasks", "check_docs_audit_scope": "pass", "changeset": "present — patch × service-messaging, plugin-webhooks", "ci": "in_progress — not polled (PM owns CI convergence, ready-flip and landing)" }, "migration_note": "The `signing_secret` column is no longer declared, so an existing database keeps it as an unmapped column still holding the old cleartext until dropped. The platform's own path covers this: `os migrate plan` reports the `drop_column` op (schema-drift `unmapped_column`, category `destructive`, so never applied unattended). Those rows also age out on the table's existing 30-day telemetry retention. Documented in the changeset and PR body, with an explicit 'rotate any exposed secret' instruction.", "other_outbox_credential_sweep": { "result": "none found", "method": "grep over every *.object.ts for a field named *secret*/*password*/*api_key*/*credential*/*signing* on objects matching outbox|delivery|queue|job — zero hits. sys_notification_delivery / receipt / inbox_message carry no credential column. The identity tables (sys_account, sys_session, sys_oauth_*) hold credentials by definition and are the auth subsystem's own channel, not outbox rows." }, "out_of_scope_findings_filed": [ { "issue": 7799, "url": "https://github.com/objectstack-ai/objectstack/issues/7799", "title": "[security] The webhook signing secret is stored in cleartext in `sys_webhook.definition_json`", "assignee": "unassigned", "why_not_here": "Not a delivery/outbox row, so outside #7722's acceptance criteria, and the fix cannot use this PR's trick — the subscriber row holds the secret precisely because FUTURE deliveries must be signed with it, so it needs a real sys_secret binding plus the schema, seeder, Studio form and row migration that implies. Folding it in would have breached the declared surface and the no-expansion instruction." } ], "open_questions": [ { "q": "Should `signature` be exposed over the data API at all?", "decided": "Yes, left readable. It is a one-way function of (body, secret) and is transmitted to the receiver in a header on every delivery, so a reader learns nothing they could not observe on the wire — while it stays genuinely useful for debugging 'was this delivery signed, and with what'. Excluding it would have cost the byte-scan test its 'guard the guard' anchor for no security gain." }, { "q": "Rotation semantics for rows already enqueued.", "decided": "A row keeps the signature matching the body it carries; rotating a subscriber's secret changes what SUBSEQUENT deliveries are signed with. This preserves the outbox's existing byte-for-byte replay contract (redeliver() already promised URL/payload untouched) and is documented in webhooks.mdx §6. A resolve-at-send-time design would have silently changed this — pending rows would start signing with the new key." }, { "q": "devDeps added to service-messaging (@objectstack/objectql, @objectstack/driver-sql).", "decided": "Accepted — dev-only, and the card's evidence bar ('reads the delivery table directly after a REAL delivery') is not reachable with the package's existing recording-fake engines. Precedent: service-automation, plugin-approvals, plugin-auth, plugin-reports all devDepend on the same pair for integration tests." } ], "blocked": false }Generated by Claude Code
test(plugin-webhooks): pin the #7722 engine double to the real write-…
huangyiirene commentedon Aug 11, 2026
Addendum — patch round 1 (PR #7798, branch
claude/issue-7722-webhook-secret-cleartext){ "issue": 7722, "pr": 7798, "patch_commit": "25e08c7", "scope": "The reported ESLint-job failure only — nothing else on the PR was touched.", "problem": "check:engine-double-contract, 2 pinned problems, both at packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts:56 — the fake engine's delete() and update() accepted every call shape, including the ones ObjectQL refuses (#4434's failure mode).", "fix": "Both verbs now open with the producer's own predicates — `assertEngineDeleteDispatch(options)` and `assertEngineUpdateDispatch(data, options)` — imported from '@objectstack/metadata-core' (the predicate's home since #5619, and the right side of the edge here), added as a devDependency of plugin-webhooks, which lacked it. The direct pin applied cleanly; no baseline entry was needed, so scripts/engine-double-contract.baseline.json is untouched.", "reproduced_locally_before_fix": "node scripts/check-engine-double-contract.mjs → 'check-engine-double-contract: 2 problem(s).', both naming this file at line 56 — same two the CI job reported.", "gates_rerun": { "check_engine_double_contract": "pass — 'OK — 152 pinned, 133 in the DEBT ledger, 2 exempt' (was 2 problems, both this file)", "plugin_webhooks_suite": "pass — 4 files / 35 tests", "eslint_changed_file": "pass — `eslint packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts --no-inline-config`, exit 0" }, "unchanged": "Fix semantics, the at-rest byte scan, and the HMAC-recompute assertions are as reported in the first report — the pinned verbs are inert members this test never calls; pinning them changes what the double would REFUSE, not what the test exercises.", "ci": "not polled — PM owns CI convergence, the ready-flip and landing." }Generated by Claude Code
huangyiirene commentedon Aug 11, 2026
ACCEPT — PR #7798 (services seat review, session
session_01JVfWSQN9RDDGwdVSmMc29x).What shipped: delivery rows carry the signature, never the key — the outbox computes
sha256=<hex>once at enqueue (the body is fixed for the row's life and replayed byte-for-byte) and drops the secret;http-senderowns both halves of the signing contract so signer and transport cannot drift. Both producers (webhook fan-out AND the Flowhttpnode — the report found the second producer the card didn't name) stop persisting cleartext with zero call-site changes.Review notes:
sys_secretciphertext rows on a 30-day telemetry table with no reaper, and wiring it needed a composition-root change outside the declared surface. This is the premise-first discipline working — the issue's own second option ("keep it off the delivery record entirely") was the stronger shape.SELECT *over every physical column after a REAL delivery (guard-the-guard anchored — the scan must also see URL + signature), HMAC recomputed over the raw received body (not "no error"), reverse verification 3/4 red on reinstated pre-fix (4th = unsigned, legitimately bidirectional).webhooks.mdx(documents this PR's own renamed column — leaving it would ship a doc naming a dead column), regenerated locale files (产物随源走), dev-only deps for the integration test (precedented in four sibling packages).webhook-signing-secret.test.tsengine double missed theassertEngineDelete/UpdateDispatchpins (check:engine-double-contract, ESLint job) — my dispatch didn't name that gate family, so the round is on the PM's ledger, not the dev's. Fixed exactly as prescribed on the same claim; all 27 checks now green.sys_webhook.definition_json#7799 (subscriber's secret insys_webhook.definition_json— needs a realsys_secretbinding, different fix class) correctly filed separately; awaiting triage.os migrate plan's destructivedrop_columnis applied by hand; 30-day retention ages the rows; "rotate any exposed secret" instruction in changeset + PR body.Marking ready and arming auto-merge now.
Generated by Claude Code
fix(service-messaging): store the delivery signature, not the signing…
fix(webhooks): stop storing the subscriber signing secret in cleartext (