Skip to content

fix(plugin-webhooks): match webhook subscriptions on the organization dimension — closes the cross-organization fan-out on both paths (#13566) - #16700

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13566-webhook-fanout-org
Sep 8, 2026
Merged

fix(plugin-webhooks): match webhook subscriptions on the organization dimension — closes the cross-organization fan-out on both paths (#13566)#16700
os-zhuang merged 4 commits into
mainfrom
claude/issue-13566-webhook-fanout-org

Conversation

@os-trump

@os-trump os-trump commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Fixes #13566

Which fan-out path this closes

Both. The per-record path (data.record.*, AutoEnqueuer.handleEvent) and the bulk path (data.records.*, AutoEnqueuer.handleBulkEvent) now match subscriptions on the organization dimension. Neither is left open on the consumer side. What remains outside this PR is producer-side and named below (carve-out objects), not a fan-out path.

The defect

AutoEnqueuer selected the subscriptions to deliver to by object name and trigger only, from one cache holding every organization's sys_webhook rows. On a walled deployment (OS_TENANCY_POSTURE=isolated|group) organization A's record events reached organization B's webhook URL, signed with B's secret, on first delivery. The census on the card (5479460377) established that a tenant's own owner/admin can create such a row through the REST data door, so this is a live cross-organization leak, p0.

The fix — one comparison, no lookup

The subscription half was already cached (CachedSubscription.organizationId, #13546); the event half is stamped by the producers — DataEvent.organizationId from the record's own tenant column (#14970 / PR #15220) and BulkDataEvent.organizationId from the Layer 0 wall's recorded verdict (#15225 / PR #15687, #15813 / PR #15878). admitsOrganization compares the two per candidate, after the trigger check and before the parked branch (a parked subscription records the payload on a dead sys_http_delivery row, so a foreign payload must not reach that either):

subscription event verdict
org A org A deliver
org A org B not a candidate (silent — the match term working)
org A absent refuse, fail-closed, warned once per subscription
none org A refuse — the ruling's case, warned once per subscription
none absent deliver — no wall on either side (every event on a single posture)

Ruling, verbatim: a subscription with no organisation ownership does not fan out — loud refusal, never a silent cross-organisation delivery.

Measured before writing the filter (what the producers actually stamp, packages/objectql/src/engine.ts on the merged tree):

  • Per record, eventOrganizationId reads the ROW's tenant column through resolveTenantFieldName — never execCtx.tenantId — and returns absent for: object not tenant-scoped, no row in hand, column absent/null/empty, malformed value. So on this path absent can also be a producer that had no row, published absent rather than substituting the caller's organization. The filter therefore treats absent as fail-closed for an organization-owned subscription on this path too, not only on the bulk path.
  • Per batch, bulkEventOrganizationId reads only the recorded TenantLayer0Verdict: organization or a one-element organizations ⇒ that organization; none, deny, a multi-membership set, no recorded verdict, or junk ⇒ absent. Absent is a routine value there (every system sweep, every multi-membership group write), and it is fail-closed: an organization-owned subscription does not take it; an organization-less (deployment-wide) subscription may, which is the consumer the spec names.

Both refusals are said once per subscription (same ledger shape as droppedForSecret, pruned to live rows on refresh), naming the consequence and the remedy, so a refused row is not dead while looking armed. An organizationId that is present but not a non-empty string drops the whole event loudly as off-contract, delivering to nobody — the schema refuses that at the publish site, so its arrival means a producer that did not validate.

Verified on the fan-out, never on delivery rows

Every new pin asserts on the refIds handed to the enqueue seam — which subscriptions the enqueuer selected. #13565 stamps a delivery with the SUBSCRIPTION's organization, so a leaked delivery reads as natively owned by the receiver; a test over sys_http_delivery rows passes on a live leak.

Reverse verification, from the committed state (9e6d20c79, HEAD blob of auto-enqueuer.ts = 2158854eb…): admitsOrganization mutated to return true (the pre-fix leak; on-disk marker count 1, blob 42d7b740… ≠ HEAD) ⇒ 8 failed | 30 passed — the leak pin, both loud-refusal pins, both fail-closed pins, the '*' pin and the say-once-ledger pin, on both paths; restored with git checkout HEAD -- PATH, marker count 0, git diff HEAD empty, git hash-object = HEAD blob (MATCH). The subject is resolved from source by the same-package test (./auto-enqueuer.js), no dist/ on the resolution path, so no dist preflight applies.

Ordered first measurement — does released 17.3.0 carry this leak?

Yes, on both fan-out paths. 17.3.0 was cut at 8a1bad8b8 (chore: version packages (#11336), 2026-09-04T10:20Z; npm @objectstack/plugin-webhooks@17.3.0 published 2026-09-04T10:53Z). Read at that commit, each zero with a control on the same tree and command shape:

⛔ No release note is written here and content/docs/releases/ is untouched — the disclosure call is the maintainer's; this measurement is its input.

Clause ② — re-derived from the export surface: no

Judged from the built dist/ of @objectstack/plugin-webhooks before and after the change (hashes in the report), not from a path test: the set of exported declarations in index.d.ts is identical (same digest), the public members of AutoEnqueuer are identical, schema.d.ts is byte-identical; the only .d.ts delta is two private member lines and comments. The change narrows the delivery set and consumes a key the producers already publish — it widens no accept set and no public surface (SKILL.md's Clause ② criterion), and a runtime security-behaviour change is by that skill's own negative boundary not Clause ②. So no needs:contract-review.

Changeset — patch on @objectstack/plugin-webhooks

Judged against AGENTS.md's rule verbatim (skip-changeset is only for a diff that publishes nothing from any released package): this diff changes the published runtime of a released package, so it takes a patch changeset (.changeset/webhook-fanout-organization-dimension.md), which states the behaviour change for organization-less subscriptions on walled deployments.

Known producer-side edge, named and left open (not widened here)

The platformGlobalObjects carve-out (#12699): #15813 landed (PR #15878, ancestor of this base) for the BULK producer — it reads the wall's recorded verdict, so a carve-out object yields none ⇒ absent ⇒ fail-closed here. For the PER-RECORD producer the carve-out is still invisible to the engine (the deployment declaration is not on the schema), so an exempted object's row may carry a stamped column value. With this filter a wrong-but-present key can only UNDER-deliver (that organization's subscriptions only; organization-less subscriptions refused loudly) — it cannot produce a cross-organization delivery — and an absent key is fail-closed for organization-owned subscriptions. Stated as reasoning from the filter's matrix, not as a measurement of a carve-out deployment; nothing here assumes the stamped key is trustworthy for carve-out objects.

验收备注

Verification (merged tree 62b1c51a2, base origin/main ba5284e60)

  • Dependency closure built (pnpm --filter '@objectstack/plugin-webhooks^...' build, 51 package builds, lock verdict command-exit 0), then pnpm --filter @objectstack/plugin-webhooks build && test && typecheck chained with &&: 12 files / 148 tests passed, tsc --noEmit + scripts tsconfig + check:test-typecheck OK. Target file alone: 38 tests (11 new pins). The public surface is byte-unchanged in every exported declaration, so no downstream consumer (pnpm --filter '...@objectstack/plugin-webhooks' direction) owes a re-run.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; derived off the merge base, identical before and after the merge): 59 families run, --ran reconciles 59 derived, 59 run, 0 unrun. 56 green. Three exited 3 (prerequisite refusal, NOT MEASURED locally, CI owns them): check:dual-build-cjs-loads and check:type-check-debt want the whole tree built (47 / 15 more packages); check:i18n wants a 10-package closure. Declared narrowing in their place: the final dist/index.cjs and dist/index.js of this package both load under node and export AutoEnqueuer (with the new filter present, 4 hits each); this diff touches no object definition or translation source, so the extractor's input for this package is unchanged; the .d.ts delta is two private members, so no ledgered package's debt can move.
  • check:doc-authoring went red on the first round (issue ids inside runtime warn strings) and is green after removing them: sibling-package prose ids hold the baseline, 829 pinned sites, no growth.
  • eslint, narrowed to the two touched files with --no-inline-config --format json: 2 files, 0 errors, 0 warnings, at 62b1c51a2. Invariance: eslint.config.mjs never enables type-aware linting for any file (its own comment at lines 326-330; the only parserOptions.project/projectService mention in the config is that comment), so this diff cannot move any untouched file's verdict; the whole-repo pnpm lint is CI's run.
  • Control bytes: grep -naP over the three changed files exits 1 (clean); check:nul-bytes green.

🤖 Generated with Claude Code

https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37


Generated by Claude Code

… dimension (#13566)

`AutoEnqueuer.handleEvent` / `handleBulkEvent` selected subscriptions by
object name and trigger only, from one cache holding every organization's
`sys_webhook` rows — so on a walled deployment organization A's record
events reached organization B's endpoint, signed with B's secret.

Both fan-out paths now compare the subscription's own organization
(`CachedSubscription.organizationId`, #13546) with the organization the
producer stamped on the event (`DataEvent.organizationId`, #14970;
`BulkDataEvent.organizationId`, #15225 / #15813): one equality per
candidate, no lookup on the hot path. A subscription with no organization
ownership does not receive an organization-walled event (loud refusal,
said once per subscription); an organization-owned subscription receives
only its own organization's events and is fail-closed on an event that
names none, on both paths. Nothing stamps either side on a `single`
posture, so delivery there is unchanged. A present-but-off-contract
`organizationId` drops the event loudly, delivering to nobody.

Pins assert on which subscriptions the enqueuer selected (the enqueue
seam), never on delivery rows — #13565 stamps a delivery with the
SUBSCRIPTION's organization, so a leaked delivery reads as natively owned
by the receiver.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…ming the subscription's organization; add the changeset

The two #13565 pins fed an organization-owned subscription an event that
named no organization — the leniency #13566 removes. The pinned assertion
(the enqueue input carries the SUBSCRIPTION's organization) is unchanged;
the event now names that same organization, so the delivery it is pinned
on still happens.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…ssage prose

`check:doc-authoring` refuses a tracker id inside customer-facing string
prose (maintainer ruling 2026-08-12); the ids stay in the code comments,
the warn/debug texts name the rule and the remedy without them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@github-actions github-actions Bot added the size/l label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-webhooks, touching 8 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx (via AutoEnqueuer (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 4 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b3ef687c043c96e928e1fde09ba973bb69e3dff8packageMentionDocs.

Which tree this was computed on

This run read content/docs from 074fb21f7c3abbcc4d178cba077fadb067a160e6 — the merge of head 62b1c51a2edd6f46b6b9141855adf634ca61e4ef into base b3ef687c043c96e928e1fde09ba973bb69e3dff8, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 074fb21f7c3abbcc4d178cba077fadb067a160e6 && git checkout 074fb21f7c3abbcc4d178cba077fadb067a160e6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b3ef687c043c96e928e1fde09ba973bb69e3dff8 62b1c51a2edd6f46b6b9141855adf634ca61e4ef && git checkout -B drift-repro b3ef687c043c96e928e1fde09ba973bb69e3dff8 && git merge --no-ff 62b1c51a2edd6f46b6b9141855adf634ca61e4ef

node scripts/docs-audit/affected-docs.mjs --json b3ef687c043c96e928e1fde09ba973bb69e3dff8

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs b3ef687c043c96e928e1fde09ba973bb69e3dff8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-zhuang
os-zhuang added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 7ceb416 Sep 8, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13566-webhook-fanout-org branch September 8, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

3 participants