Skip to content

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows - #13565

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id
Aug 31, 2026
Merged

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows#13565
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#13546

What breaks today

sys_http_delivery is tenant-scoped and redeliver() — its one request-reachable door (POST /api/v1/webhooks/redeliver, open to any authenticated user) — deliberately scopes by the caller's organization (#10740). But the enqueue door never stamped organization_id, and the SQL driver's tenant term is (organization_id = :tenantId OR organization_id IS NULL) — a deliberate global-row fail-open. So 100% of rows landed in the NULL arm: visible to, and replayable by, every organization on a walled deployment. Declared, not enforced; this PR restores the declared contract. The driver's tenant term is untouched — removing the IS NULL arm would hide true global rows from every tenant, a different defect.

The repair — mirror of the sibling that already received it

The notification outbox got exactly this fix (EnqueueDeliveryInput.organizationId, SqlOutbox.enqueue writing organization_id: input.organizationId ?? null). This PR gives the HTTP outbox the same shape, end to end:

Forward-stamping only. Existing NULL rows are untouched; their disposition is the separate decision the card names. No backfill, no driver changes, no schema changes (organization_id is kernel-provisioned on every object).

Optional vs required — argued, not assumed

The new member is optional (organizationId?: string). Weighed explicitly against the required-but-undefined-able shape this same file uses for RedeliverOptions.tenantId:

  • For optional: it mirrors the sibling notification outbox's EnqueueDeliveryInput.organizationId and EmitInput.organizationId — one convention across the two outboxes of one package (a second convention here would be this card's defect repeated). It is non-breaking for the published surface. And org-less enqueues are legitimate: a single-posture install and every stack before its first organization must keep enqueuing.
  • For required (organizationId: string | undefined): nothing tells a future producer it forgot one — the exact gap that produced this bug. That is a real cost. The mitigations shipped here: the producer population of this seam is enumerated (two, both repaired in this PR), the flow node warns loudly on an org-less multi-org run, and the driver's auditMissingTenant line still reports unscoped writes on multi-tenant boots. RedeliverOptions.tenantId went required because its callers are arbitrary request routes; this seam's callers are the two service-internal producers.

If the contract reviewer prefers the required shape, the change is mechanical (both producers already always spell the key or spread it conditionally) but it becomes a compile-breaking widening of a published input.

Write-surface enumeration (the triage's must-answer)

Is SqlHttpOutbox.insert the only door? Enumerated, not exemplified, at origin/main (ff37576):

  • Production constructors of an IHttpOutbox: exactly one — messaging-service-plugin.ts:310 (new SqlHttpOutbox(engine, ...), default object name). MemoryHttpOutbox is constructed only by tests.
  • Row-creating engine calls on the object: exactly one INSERT site, SqlHttpOutbox.insert, reached from enqueue() and recordUndeliverable() — both covered by this stamp. No bulkCreate/upsert/seed writes anywhere (grep -rn "insert(SYS_HTTP_DELIVERY|'sys_http_delivery'" over packages/ apps/: only the four test files that build their own rows).
  • The remaining writes (claim/ack/redeliver UPDATEs) create no rows and never touch the tenant column; userActions declares create/edit/delete false and the object is managedBy: 'engine-owned'.

Verification

  • New pins: packages/services/service-messaging/src/http-outbox-organization.test.ts (9 — SQL stamp verbatim / explicit-NULL normalization / parked-row stamp / read-back mapping; memory parity + all three arms of the redeliver predicate, ADR-0112 code asserted on the refusal); http-nodes.test.ts (+2 — tenant threaded verbatim; over-denial control: org-less run still enqueues, key absent, warns); auto-enqueuer.test.ts (+3 — per-record, org-less control, bulk path).
  • Targeted suites at head sha e2ecb71: service-messaging 7 files / 48 tests pass (incl. the tenant-audit and parked-record integration suites), service-automation 4 files / 34 pass, plugin-webhooks 3 files / 40 pass. typecheck green for service-messaging + plugin-webhooks (service-automation declares none); --listFiles confirms the new test file is inside the messaging typecheck.
  • Reverse verification (committed state, restore proven by blob hash vs HEAD): plugin-webhooks resolves @objectstack/service-messaging via dist/index.d.ts (tsc --listFiles); mutating the producer key to organizationIdX reddens typecheck with TS2561 naming organizationId as the fix — proving the rebuilt .d.ts carries the member and the literal is enforced against it. Restore verified byte-identical (worktree blob = HEAD blob 7f52692).
  • Gates at e2ecb71: all 14 path-derived families green except check:dual-build-cjs-loads, which is NOT MEASURED locally (PREREQUISITE NOT MET — it reads every package's dist and 41+ untouched packages are unbuilt here; CI's Build Core owns the full run). Declared narrowing: the gate's own population is every published package's built entry (quoted from its error output); this diff touches 3 packages and no build/exports config of any other, so untouched packages' verdicts cannot move; the 3 touched packages were rebuilt and their CJS entries require() clean (3/3 probed).

Generated by Claude Code


Generated by Claude Code

…ws (#13546)
Part of #13546 — the enqueue door never wrote the tenant column, so every
row landed in the SQL driver's (organization_id IS NULL) global-row arm and
the cross-organization wall on redeliver() (#10740) excluded nothing.
Mirrors the notification outbox's existing repair (EnqueueDeliveryInput):
- EnqueueHttpInput gains an optional organizationId (inherited by
UndeliverableHttpInput, so parked rows are stamped too); HttpDelivery
surfaces it on read-back.
- SqlHttpOutbox.insert writes organization_id: input.organizationId ?? null,
the same line SqlOutbox.enqueue writes.
- MemoryHttpOutbox stores the field and, now that its rows carry a tenant,
applies RedeliverOptions.tenantId in redeliver() with the driver's exact
semantics (other org invisible; org-less row global; tenant-less caller
unscoped).
- The flow http node (durable mode) threads AutomationContext.tenantId —
the notify node's #11303 source — and warns loudly when a run has none.
- The webhook auto-enqueuer stamps each delivery with its subscription's
own organization (sys_webhook.organization_id).
Forward-stamping only; existing NULL rows are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…string
check:doc-authoring red: operators cannot resolve #NNNN. The anchor stays
in the adjacent code comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-webhooks, @objectstack/service-automation, @objectstack/service-messaging, touching 13 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (4 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 214ce1a01bdf23e06de3ef9a877c3a36013531fd — the merge of head 479e6315071c3b8cef710d46da7e64b4d3a160c1 into base 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0, 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 214ce1a01bdf23e06de3ef9a877c3a36013531fd && git checkout 214ce1a01bdf23e06de3ef9a877c3a36013531fd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 479e6315071c3b8cef710d46da7e64b4d3a160c1 && git checkout -B drift-repro 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 && git merge --no-ff 479e6315071c3b8cef710d46da7e64b4d3a160c1
node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0

⚠️ 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… ledger
check-engine-double-contract --write: 1 row added (findOne, pinned: 1),
0 lost — additive ratchet only, per the gate's own instruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_http_delivery rows always land organization_id = NULL, so the deliberate cross-organization wall on redeliver() is reachable around for every row

3 participants

@os-steve@huangyiirene@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows by os-steve · Pull Request #13565 · objectstack-ai/objectstack · GitHub
Skip to content

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows - #13565

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id
Aug 31, 2026
Merged

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows#13565
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#13546

What breaks today

sys_http_delivery is tenant-scoped and redeliver() — its one request-reachable door (POST /api/v1/webhooks/redeliver, open to any authenticated user) — deliberately scopes by the caller's organization (#10740). But the enqueue door never stamped organization_id, and the SQL driver's tenant term is (organization_id = :tenantId OR organization_id IS NULL) — a deliberate global-row fail-open. So 100% of rows landed in the NULL arm: visible to, and replayable by, every organization on a walled deployment. Declared, not enforced; this PR restores the declared contract. The driver's tenant term is untouched — removing the IS NULL arm would hide true global rows from every tenant, a different defect.

The repair — mirror of the sibling that already received it

The notification outbox got exactly this fix (EnqueueDeliveryInput.organizationId, SqlOutbox.enqueue writing organization_id: input.organizationId ?? null). This PR gives the HTTP outbox the same shape, end to end:

Forward-stamping only. Existing NULL rows are untouched; their disposition is the separate decision the card names. No backfill, no driver changes, no schema changes (organization_id is kernel-provisioned on every object).

Optional vs required — argued, not assumed

The new member is optional (organizationId?: string). Weighed explicitly against the required-but-undefined-able shape this same file uses for RedeliverOptions.tenantId:

  • For optional: it mirrors the sibling notification outbox's EnqueueDeliveryInput.organizationId and EmitInput.organizationId — one convention across the two outboxes of one package (a second convention here would be this card's defect repeated). It is non-breaking for the published surface. And org-less enqueues are legitimate: a single-posture install and every stack before its first organization must keep enqueuing.
  • For required (organizationId: string | undefined): nothing tells a future producer it forgot one — the exact gap that produced this bug. That is a real cost. The mitigations shipped here: the producer population of this seam is enumerated (two, both repaired in this PR), the flow node warns loudly on an org-less multi-org run, and the driver's auditMissingTenant line still reports unscoped writes on multi-tenant boots. RedeliverOptions.tenantId went required because its callers are arbitrary request routes; this seam's callers are the two service-internal producers.

If the contract reviewer prefers the required shape, the change is mechanical (both producers already always spell the key or spread it conditionally) but it becomes a compile-breaking widening of a published input.

Write-surface enumeration (the triage's must-answer)

Is SqlHttpOutbox.insert the only door? Enumerated, not exemplified, at origin/main (ff37576):

  • Production constructors of an IHttpOutbox: exactly one — messaging-service-plugin.ts:310 (new SqlHttpOutbox(engine, ...), default object name). MemoryHttpOutbox is constructed only by tests.
  • Row-creating engine calls on the object: exactly one INSERT site, SqlHttpOutbox.insert, reached from enqueue() and recordUndeliverable() — both covered by this stamp. No bulkCreate/upsert/seed writes anywhere (grep -rn "insert(SYS_HTTP_DELIVERY|'sys_http_delivery'" over packages/ apps/: only the four test files that build their own rows).
  • The remaining writes (claim/ack/redeliver UPDATEs) create no rows and never touch the tenant column; userActions declares create/edit/delete false and the object is managedBy: 'engine-owned'.

Verification

  • New pins: packages/services/service-messaging/src/http-outbox-organization.test.ts (9 — SQL stamp verbatim / explicit-NULL normalization / parked-row stamp / read-back mapping; memory parity + all three arms of the redeliver predicate, ADR-0112 code asserted on the refusal); http-nodes.test.ts (+2 — tenant threaded verbatim; over-denial control: org-less run still enqueues, key absent, warns); auto-enqueuer.test.ts (+3 — per-record, org-less control, bulk path).
  • Targeted suites at head sha e2ecb71: service-messaging 7 files / 48 tests pass (incl. the tenant-audit and parked-record integration suites), service-automation 4 files / 34 pass, plugin-webhooks 3 files / 40 pass. typecheck green for service-messaging + plugin-webhooks (service-automation declares none); --listFiles confirms the new test file is inside the messaging typecheck.
  • Reverse verification (committed state, restore proven by blob hash vs HEAD): plugin-webhooks resolves @objectstack/service-messaging via dist/index.d.ts (tsc --listFiles); mutating the producer key to organizationIdX reddens typecheck with TS2561 naming organizationId as the fix — proving the rebuilt .d.ts carries the member and the literal is enforced against it. Restore verified byte-identical (worktree blob = HEAD blob 7f52692).
  • Gates at e2ecb71: all 14 path-derived families green except check:dual-build-cjs-loads, which is NOT MEASURED locally (PREREQUISITE NOT MET — it reads every package's dist and 41+ untouched packages are unbuilt here; CI's Build Core owns the full run). Declared narrowing: the gate's own population is every published package's built entry (quoted from its error output); this diff touches 3 packages and no build/exports config of any other, so untouched packages' verdicts cannot move; the 3 touched packages were rebuilt and their CJS entries require() clean (3/3 probed).

Generated by Claude Code


Generated by Claude Code

…ws (#13546)
Part of #13546 — the enqueue door never wrote the tenant column, so every
row landed in the SQL driver's (organization_id IS NULL) global-row arm and
the cross-organization wall on redeliver() (#10740) excluded nothing.
Mirrors the notification outbox's existing repair (EnqueueDeliveryInput):
- EnqueueHttpInput gains an optional organizationId (inherited by
UndeliverableHttpInput, so parked rows are stamped too); HttpDelivery
surfaces it on read-back.
- SqlHttpOutbox.insert writes organization_id: input.organizationId ?? null,
the same line SqlOutbox.enqueue writes.
- MemoryHttpOutbox stores the field and, now that its rows carry a tenant,
applies RedeliverOptions.tenantId in redeliver() with the driver's exact
semantics (other org invisible; org-less row global; tenant-less caller
unscoped).
- The flow http node (durable mode) threads AutomationContext.tenantId —
the notify node's #11303 source — and warns loudly when a run has none.
- The webhook auto-enqueuer stamps each delivery with its subscription's
own organization (sys_webhook.organization_id).
Forward-stamping only; existing NULL rows are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…string
check:doc-authoring red: operators cannot resolve #NNNN. The anchor stays
in the adjacent code comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-webhooks, @objectstack/service-automation, @objectstack/service-messaging, touching 13 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (4 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 214ce1a01bdf23e06de3ef9a877c3a36013531fd — the merge of head 479e6315071c3b8cef710d46da7e64b4d3a160c1 into base 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0, 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 214ce1a01bdf23e06de3ef9a877c3a36013531fd && git checkout 214ce1a01bdf23e06de3ef9a877c3a36013531fd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 479e6315071c3b8cef710d46da7e64b4d3a160c1 && git checkout -B drift-repro 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 && git merge --no-ff 479e6315071c3b8cef710d46da7e64b4d3a160c1
node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0

⚠️ 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… ledger
check-engine-double-contract --write: 1 row added (findOne, pinned: 1),
0 lost — additive ratchet only, per the gate's own instruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_http_delivery rows always land organization_id = NULL, so the deliberate cross-organization wall on redeliver() is reachable around for every row

3 participants

@os-steve@huangyiirene@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows by os-steve · Pull Request #13565 · objectstack-ai/objectstack · GitHub
Skip to content

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows - #13565

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id
Aug 31, 2026
Merged

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows#13565
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#13546

What breaks today

sys_http_delivery is tenant-scoped and redeliver() — its one request-reachable door (POST /api/v1/webhooks/redeliver, open to any authenticated user) — deliberately scopes by the caller's organization (#10740). But the enqueue door never stamped organization_id, and the SQL driver's tenant term is (organization_id = :tenantId OR organization_id IS NULL) — a deliberate global-row fail-open. So 100% of rows landed in the NULL arm: visible to, and replayable by, every organization on a walled deployment. Declared, not enforced; this PR restores the declared contract. The driver's tenant term is untouched — removing the IS NULL arm would hide true global rows from every tenant, a different defect.

The repair — mirror of the sibling that already received it

The notification outbox got exactly this fix (EnqueueDeliveryInput.organizationId, SqlOutbox.enqueue writing organization_id: input.organizationId ?? null). This PR gives the HTTP outbox the same shape, end to end:

Forward-stamping only. Existing NULL rows are untouched; their disposition is the separate decision the card names. No backfill, no driver changes, no schema changes (organization_id is kernel-provisioned on every object).

Optional vs required — argued, not assumed

The new member is optional (organizationId?: string). Weighed explicitly against the required-but-undefined-able shape this same file uses for RedeliverOptions.tenantId:

  • For optional: it mirrors the sibling notification outbox's EnqueueDeliveryInput.organizationId and EmitInput.organizationId — one convention across the two outboxes of one package (a second convention here would be this card's defect repeated). It is non-breaking for the published surface. And org-less enqueues are legitimate: a single-posture install and every stack before its first organization must keep enqueuing.
  • For required (organizationId: string | undefined): nothing tells a future producer it forgot one — the exact gap that produced this bug. That is a real cost. The mitigations shipped here: the producer population of this seam is enumerated (two, both repaired in this PR), the flow node warns loudly on an org-less multi-org run, and the driver's auditMissingTenant line still reports unscoped writes on multi-tenant boots. RedeliverOptions.tenantId went required because its callers are arbitrary request routes; this seam's callers are the two service-internal producers.

If the contract reviewer prefers the required shape, the change is mechanical (both producers already always spell the key or spread it conditionally) but it becomes a compile-breaking widening of a published input.

Write-surface enumeration (the triage's must-answer)

Is SqlHttpOutbox.insert the only door? Enumerated, not exemplified, at origin/main (ff37576):

  • Production constructors of an IHttpOutbox: exactly one — messaging-service-plugin.ts:310 (new SqlHttpOutbox(engine, ...), default object name). MemoryHttpOutbox is constructed only by tests.
  • Row-creating engine calls on the object: exactly one INSERT site, SqlHttpOutbox.insert, reached from enqueue() and recordUndeliverable() — both covered by this stamp. No bulkCreate/upsert/seed writes anywhere (grep -rn "insert(SYS_HTTP_DELIVERY|'sys_http_delivery'" over packages/ apps/: only the four test files that build their own rows).
  • The remaining writes (claim/ack/redeliver UPDATEs) create no rows and never touch the tenant column; userActions declares create/edit/delete false and the object is managedBy: 'engine-owned'.

Verification

  • New pins: packages/services/service-messaging/src/http-outbox-organization.test.ts (9 — SQL stamp verbatim / explicit-NULL normalization / parked-row stamp / read-back mapping; memory parity + all three arms of the redeliver predicate, ADR-0112 code asserted on the refusal); http-nodes.test.ts (+2 — tenant threaded verbatim; over-denial control: org-less run still enqueues, key absent, warns); auto-enqueuer.test.ts (+3 — per-record, org-less control, bulk path).
  • Targeted suites at head sha e2ecb71: service-messaging 7 files / 48 tests pass (incl. the tenant-audit and parked-record integration suites), service-automation 4 files / 34 pass, plugin-webhooks 3 files / 40 pass. typecheck green for service-messaging + plugin-webhooks (service-automation declares none); --listFiles confirms the new test file is inside the messaging typecheck.
  • Reverse verification (committed state, restore proven by blob hash vs HEAD): plugin-webhooks resolves @objectstack/service-messaging via dist/index.d.ts (tsc --listFiles); mutating the producer key to organizationIdX reddens typecheck with TS2561 naming organizationId as the fix — proving the rebuilt .d.ts carries the member and the literal is enforced against it. Restore verified byte-identical (worktree blob = HEAD blob 7f52692).
  • Gates at e2ecb71: all 14 path-derived families green except check:dual-build-cjs-loads, which is NOT MEASURED locally (PREREQUISITE NOT MET — it reads every package's dist and 41+ untouched packages are unbuilt here; CI's Build Core owns the full run). Declared narrowing: the gate's own population is every published package's built entry (quoted from its error output); this diff touches 3 packages and no build/exports config of any other, so untouched packages' verdicts cannot move; the 3 touched packages were rebuilt and their CJS entries require() clean (3/3 probed).

Generated by Claude Code


Generated by Claude Code

…ws (#13546)
Part of #13546 — the enqueue door never wrote the tenant column, so every
row landed in the SQL driver's (organization_id IS NULL) global-row arm and
the cross-organization wall on redeliver() (#10740) excluded nothing.
Mirrors the notification outbox's existing repair (EnqueueDeliveryInput):
- EnqueueHttpInput gains an optional organizationId (inherited by
UndeliverableHttpInput, so parked rows are stamped too); HttpDelivery
surfaces it on read-back.
- SqlHttpOutbox.insert writes organization_id: input.organizationId ?? null,
the same line SqlOutbox.enqueue writes.
- MemoryHttpOutbox stores the field and, now that its rows carry a tenant,
applies RedeliverOptions.tenantId in redeliver() with the driver's exact
semantics (other org invisible; org-less row global; tenant-less caller
unscoped).
- The flow http node (durable mode) threads AutomationContext.tenantId —
the notify node's #11303 source — and warns loudly when a run has none.
- The webhook auto-enqueuer stamps each delivery with its subscription's
own organization (sys_webhook.organization_id).
Forward-stamping only; existing NULL rows are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…string
check:doc-authoring red: operators cannot resolve #NNNN. The anchor stays
in the adjacent code comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-webhooks, @objectstack/service-automation, @objectstack/service-messaging, touching 13 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (4 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 214ce1a01bdf23e06de3ef9a877c3a36013531fd — the merge of head 479e6315071c3b8cef710d46da7e64b4d3a160c1 into base 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0, 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 214ce1a01bdf23e06de3ef9a877c3a36013531fd && git checkout 214ce1a01bdf23e06de3ef9a877c3a36013531fd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 479e6315071c3b8cef710d46da7e64b4d3a160c1 && git checkout -B drift-repro 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 && git merge --no-ff 479e6315071c3b8cef710d46da7e64b4d3a160c1
node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0

⚠️ 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… ledger
check-engine-double-contract --write: 1 row added (findOne, pinned: 1),
0 lost — additive ratchet only, per the gate's own instruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_http_delivery rows always land organization_id = NULL, so the deliberate cross-organization wall on redeliver() is reachable around for every row

3 participants

@os-steve@huangyiirene@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows by os-steve · Pull Request #13565 · objectstack-ai/objectstack · GitHub
Skip to content

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows - #13565

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id
Aug 31, 2026
Merged

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows#13565
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#13546

What breaks today

sys_http_delivery is tenant-scoped and redeliver() — its one request-reachable door (POST /api/v1/webhooks/redeliver, open to any authenticated user) — deliberately scopes by the caller's organization (#10740). But the enqueue door never stamped organization_id, and the SQL driver's tenant term is (organization_id = :tenantId OR organization_id IS NULL) — a deliberate global-row fail-open. So 100% of rows landed in the NULL arm: visible to, and replayable by, every organization on a walled deployment. Declared, not enforced; this PR restores the declared contract. The driver's tenant term is untouched — removing the IS NULL arm would hide true global rows from every tenant, a different defect.

The repair — mirror of the sibling that already received it

The notification outbox got exactly this fix (EnqueueDeliveryInput.organizationId, SqlOutbox.enqueue writing organization_id: input.organizationId ?? null). This PR gives the HTTP outbox the same shape, end to end:

Forward-stamping only. Existing NULL rows are untouched; their disposition is the separate decision the card names. No backfill, no driver changes, no schema changes (organization_id is kernel-provisioned on every object).

Optional vs required — argued, not assumed

The new member is optional (organizationId?: string). Weighed explicitly against the required-but-undefined-able shape this same file uses for RedeliverOptions.tenantId:

  • For optional: it mirrors the sibling notification outbox's EnqueueDeliveryInput.organizationId and EmitInput.organizationId — one convention across the two outboxes of one package (a second convention here would be this card's defect repeated). It is non-breaking for the published surface. And org-less enqueues are legitimate: a single-posture install and every stack before its first organization must keep enqueuing.
  • For required (organizationId: string | undefined): nothing tells a future producer it forgot one — the exact gap that produced this bug. That is a real cost. The mitigations shipped here: the producer population of this seam is enumerated (two, both repaired in this PR), the flow node warns loudly on an org-less multi-org run, and the driver's auditMissingTenant line still reports unscoped writes on multi-tenant boots. RedeliverOptions.tenantId went required because its callers are arbitrary request routes; this seam's callers are the two service-internal producers.

If the contract reviewer prefers the required shape, the change is mechanical (both producers already always spell the key or spread it conditionally) but it becomes a compile-breaking widening of a published input.

Write-surface enumeration (the triage's must-answer)

Is SqlHttpOutbox.insert the only door? Enumerated, not exemplified, at origin/main (ff37576):

  • Production constructors of an IHttpOutbox: exactly one — messaging-service-plugin.ts:310 (new SqlHttpOutbox(engine, ...), default object name). MemoryHttpOutbox is constructed only by tests.
  • Row-creating engine calls on the object: exactly one INSERT site, SqlHttpOutbox.insert, reached from enqueue() and recordUndeliverable() — both covered by this stamp. No bulkCreate/upsert/seed writes anywhere (grep -rn "insert(SYS_HTTP_DELIVERY|'sys_http_delivery'" over packages/ apps/: only the four test files that build their own rows).
  • The remaining writes (claim/ack/redeliver UPDATEs) create no rows and never touch the tenant column; userActions declares create/edit/delete false and the object is managedBy: 'engine-owned'.

Verification

  • New pins: packages/services/service-messaging/src/http-outbox-organization.test.ts (9 — SQL stamp verbatim / explicit-NULL normalization / parked-row stamp / read-back mapping; memory parity + all three arms of the redeliver predicate, ADR-0112 code asserted on the refusal); http-nodes.test.ts (+2 — tenant threaded verbatim; over-denial control: org-less run still enqueues, key absent, warns); auto-enqueuer.test.ts (+3 — per-record, org-less control, bulk path).
  • Targeted suites at head sha e2ecb71: service-messaging 7 files / 48 tests pass (incl. the tenant-audit and parked-record integration suites), service-automation 4 files / 34 pass, plugin-webhooks 3 files / 40 pass. typecheck green for service-messaging + plugin-webhooks (service-automation declares none); --listFiles confirms the new test file is inside the messaging typecheck.
  • Reverse verification (committed state, restore proven by blob hash vs HEAD): plugin-webhooks resolves @objectstack/service-messaging via dist/index.d.ts (tsc --listFiles); mutating the producer key to organizationIdX reddens typecheck with TS2561 naming organizationId as the fix — proving the rebuilt .d.ts carries the member and the literal is enforced against it. Restore verified byte-identical (worktree blob = HEAD blob 7f52692).
  • Gates at e2ecb71: all 14 path-derived families green except check:dual-build-cjs-loads, which is NOT MEASURED locally (PREREQUISITE NOT MET — it reads every package's dist and 41+ untouched packages are unbuilt here; CI's Build Core owns the full run). Declared narrowing: the gate's own population is every published package's built entry (quoted from its error output); this diff touches 3 packages and no build/exports config of any other, so untouched packages' verdicts cannot move; the 3 touched packages were rebuilt and their CJS entries require() clean (3/3 probed).

Generated by Claude Code


Generated by Claude Code

…ws (#13546)
Part of #13546 — the enqueue door never wrote the tenant column, so every
row landed in the SQL driver's (organization_id IS NULL) global-row arm and
the cross-organization wall on redeliver() (#10740) excluded nothing.
Mirrors the notification outbox's existing repair (EnqueueDeliveryInput):
- EnqueueHttpInput gains an optional organizationId (inherited by
UndeliverableHttpInput, so parked rows are stamped too); HttpDelivery
surfaces it on read-back.
- SqlHttpOutbox.insert writes organization_id: input.organizationId ?? null,
the same line SqlOutbox.enqueue writes.
- MemoryHttpOutbox stores the field and, now that its rows carry a tenant,
applies RedeliverOptions.tenantId in redeliver() with the driver's exact
semantics (other org invisible; org-less row global; tenant-less caller
unscoped).
- The flow http node (durable mode) threads AutomationContext.tenantId —
the notify node's #11303 source — and warns loudly when a run has none.
- The webhook auto-enqueuer stamps each delivery with its subscription's
own organization (sys_webhook.organization_id).
Forward-stamping only; existing NULL rows are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…string
check:doc-authoring red: operators cannot resolve #NNNN. The anchor stays
in the adjacent code comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-webhooks, @objectstack/service-automation, @objectstack/service-messaging, touching 13 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (4 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 214ce1a01bdf23e06de3ef9a877c3a36013531fd — the merge of head 479e6315071c3b8cef710d46da7e64b4d3a160c1 into base 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0, 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 214ce1a01bdf23e06de3ef9a877c3a36013531fd && git checkout 214ce1a01bdf23e06de3ef9a877c3a36013531fd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 479e6315071c3b8cef710d46da7e64b4d3a160c1 && git checkout -B drift-repro 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 && git merge --no-ff 479e6315071c3b8cef710d46da7e64b4d3a160c1
node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0

⚠️ 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… ledger
check-engine-double-contract --write: 1 row added (findOne, pinned: 1),
0 lost — additive ratchet only, per the gate's own instruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_http_delivery rows always land organization_id = NULL, so the deliberate cross-organization wall on redeliver() is reachable around for every row

3 participants

@os-steve@huangyiirene@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows by os-steve · Pull Request #13565 · objectstack-ai/objectstack · GitHub
Skip to content

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows - #13565

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id
Aug 31, 2026
Merged

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows#13565
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#13546

What breaks today

sys_http_delivery is tenant-scoped and redeliver() — its one request-reachable door (POST /api/v1/webhooks/redeliver, open to any authenticated user) — deliberately scopes by the caller's organization (#10740). But the enqueue door never stamped organization_id, and the SQL driver's tenant term is (organization_id = :tenantId OR organization_id IS NULL) — a deliberate global-row fail-open. So 100% of rows landed in the NULL arm: visible to, and replayable by, every organization on a walled deployment. Declared, not enforced; this PR restores the declared contract. The driver's tenant term is untouched — removing the IS NULL arm would hide true global rows from every tenant, a different defect.

The repair — mirror of the sibling that already received it

The notification outbox got exactly this fix (EnqueueDeliveryInput.organizationId, SqlOutbox.enqueue writing organization_id: input.organizationId ?? null). This PR gives the HTTP outbox the same shape, end to end:

Forward-stamping only. Existing NULL rows are untouched; their disposition is the separate decision the card names. No backfill, no driver changes, no schema changes (organization_id is kernel-provisioned on every object).

Optional vs required — argued, not assumed

The new member is optional (organizationId?: string). Weighed explicitly against the required-but-undefined-able shape this same file uses for RedeliverOptions.tenantId:

  • For optional: it mirrors the sibling notification outbox's EnqueueDeliveryInput.organizationId and EmitInput.organizationId — one convention across the two outboxes of one package (a second convention here would be this card's defect repeated). It is non-breaking for the published surface. And org-less enqueues are legitimate: a single-posture install and every stack before its first organization must keep enqueuing.
  • For required (organizationId: string | undefined): nothing tells a future producer it forgot one — the exact gap that produced this bug. That is a real cost. The mitigations shipped here: the producer population of this seam is enumerated (two, both repaired in this PR), the flow node warns loudly on an org-less multi-org run, and the driver's auditMissingTenant line still reports unscoped writes on multi-tenant boots. RedeliverOptions.tenantId went required because its callers are arbitrary request routes; this seam's callers are the two service-internal producers.

If the contract reviewer prefers the required shape, the change is mechanical (both producers already always spell the key or spread it conditionally) but it becomes a compile-breaking widening of a published input.

Write-surface enumeration (the triage's must-answer)

Is SqlHttpOutbox.insert the only door? Enumerated, not exemplified, at origin/main (ff37576):

  • Production constructors of an IHttpOutbox: exactly one — messaging-service-plugin.ts:310 (new SqlHttpOutbox(engine, ...), default object name). MemoryHttpOutbox is constructed only by tests.
  • Row-creating engine calls on the object: exactly one INSERT site, SqlHttpOutbox.insert, reached from enqueue() and recordUndeliverable() — both covered by this stamp. No bulkCreate/upsert/seed writes anywhere (grep -rn "insert(SYS_HTTP_DELIVERY|'sys_http_delivery'" over packages/ apps/: only the four test files that build their own rows).
  • The remaining writes (claim/ack/redeliver UPDATEs) create no rows and never touch the tenant column; userActions declares create/edit/delete false and the object is managedBy: 'engine-owned'.

Verification

  • New pins: packages/services/service-messaging/src/http-outbox-organization.test.ts (9 — SQL stamp verbatim / explicit-NULL normalization / parked-row stamp / read-back mapping; memory parity + all three arms of the redeliver predicate, ADR-0112 code asserted on the refusal); http-nodes.test.ts (+2 — tenant threaded verbatim; over-denial control: org-less run still enqueues, key absent, warns); auto-enqueuer.test.ts (+3 — per-record, org-less control, bulk path).
  • Targeted suites at head sha e2ecb71: service-messaging 7 files / 48 tests pass (incl. the tenant-audit and parked-record integration suites), service-automation 4 files / 34 pass, plugin-webhooks 3 files / 40 pass. typecheck green for service-messaging + plugin-webhooks (service-automation declares none); --listFiles confirms the new test file is inside the messaging typecheck.
  • Reverse verification (committed state, restore proven by blob hash vs HEAD): plugin-webhooks resolves @objectstack/service-messaging via dist/index.d.ts (tsc --listFiles); mutating the producer key to organizationIdX reddens typecheck with TS2561 naming organizationId as the fix — proving the rebuilt .d.ts carries the member and the literal is enforced against it. Restore verified byte-identical (worktree blob = HEAD blob 7f52692).
  • Gates at e2ecb71: all 14 path-derived families green except check:dual-build-cjs-loads, which is NOT MEASURED locally (PREREQUISITE NOT MET — it reads every package's dist and 41+ untouched packages are unbuilt here; CI's Build Core owns the full run). Declared narrowing: the gate's own population is every published package's built entry (quoted from its error output); this diff touches 3 packages and no build/exports config of any other, so untouched packages' verdicts cannot move; the 3 touched packages were rebuilt and their CJS entries require() clean (3/3 probed).

Generated by Claude Code


Generated by Claude Code

…ws (#13546)
Part of #13546 — the enqueue door never wrote the tenant column, so every
row landed in the SQL driver's (organization_id IS NULL) global-row arm and
the cross-organization wall on redeliver() (#10740) excluded nothing.
Mirrors the notification outbox's existing repair (EnqueueDeliveryInput):
- EnqueueHttpInput gains an optional organizationId (inherited by
UndeliverableHttpInput, so parked rows are stamped too); HttpDelivery
surfaces it on read-back.
- SqlHttpOutbox.insert writes organization_id: input.organizationId ?? null,
the same line SqlOutbox.enqueue writes.
- MemoryHttpOutbox stores the field and, now that its rows carry a tenant,
applies RedeliverOptions.tenantId in redeliver() with the driver's exact
semantics (other org invisible; org-less row global; tenant-less caller
unscoped).
- The flow http node (durable mode) threads AutomationContext.tenantId —
the notify node's #11303 source — and warns loudly when a run has none.
- The webhook auto-enqueuer stamps each delivery with its subscription's
own organization (sys_webhook.organization_id).
Forward-stamping only; existing NULL rows are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…string
check:doc-authoring red: operators cannot resolve #NNNN. The anchor stays
in the adjacent code comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-webhooks, @objectstack/service-automation, @objectstack/service-messaging, touching 13 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (4 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 214ce1a01bdf23e06de3ef9a877c3a36013531fd — the merge of head 479e6315071c3b8cef710d46da7e64b4d3a160c1 into base 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0, 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 214ce1a01bdf23e06de3ef9a877c3a36013531fd && git checkout 214ce1a01bdf23e06de3ef9a877c3a36013531fd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 479e6315071c3b8cef710d46da7e64b4d3a160c1 && git checkout -B drift-repro 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 && git merge --no-ff 479e6315071c3b8cef710d46da7e64b4d3a160c1
node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0

⚠️ 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… ledger
check-engine-double-contract --write: 1 row added (findOne, pinned: 1),
0 lost — additive ratchet only, per the gate's own instruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_http_delivery rows always land organization_id = NULL, so the deliberate cross-organization wall on redeliver() is reachable around for every row

3 participants

@os-steve@huangyiirene@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows by os-steve · Pull Request #13565 · objectstack-ai/objectstack · GitHub
Skip to content

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows - #13565

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id
Aug 31, 2026
Merged

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows#13565
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#13546

What breaks today

sys_http_delivery is tenant-scoped and redeliver() — its one request-reachable door (POST /api/v1/webhooks/redeliver, open to any authenticated user) — deliberately scopes by the caller's organization (#10740). But the enqueue door never stamped organization_id, and the SQL driver's tenant term is (organization_id = :tenantId OR organization_id IS NULL) — a deliberate global-row fail-open. So 100% of rows landed in the NULL arm: visible to, and replayable by, every organization on a walled deployment. Declared, not enforced; this PR restores the declared contract. The driver's tenant term is untouched — removing the IS NULL arm would hide true global rows from every tenant, a different defect.

The repair — mirror of the sibling that already received it

The notification outbox got exactly this fix (EnqueueDeliveryInput.organizationId, SqlOutbox.enqueue writing organization_id: input.organizationId ?? null). This PR gives the HTTP outbox the same shape, end to end:

Forward-stamping only. Existing NULL rows are untouched; their disposition is the separate decision the card names. No backfill, no driver changes, no schema changes (organization_id is kernel-provisioned on every object).

Optional vs required — argued, not assumed

The new member is optional (organizationId?: string). Weighed explicitly against the required-but-undefined-able shape this same file uses for RedeliverOptions.tenantId:

  • For optional: it mirrors the sibling notification outbox's EnqueueDeliveryInput.organizationId and EmitInput.organizationId — one convention across the two outboxes of one package (a second convention here would be this card's defect repeated). It is non-breaking for the published surface. And org-less enqueues are legitimate: a single-posture install and every stack before its first organization must keep enqueuing.
  • For required (organizationId: string | undefined): nothing tells a future producer it forgot one — the exact gap that produced this bug. That is a real cost. The mitigations shipped here: the producer population of this seam is enumerated (two, both repaired in this PR), the flow node warns loudly on an org-less multi-org run, and the driver's auditMissingTenant line still reports unscoped writes on multi-tenant boots. RedeliverOptions.tenantId went required because its callers are arbitrary request routes; this seam's callers are the two service-internal producers.

If the contract reviewer prefers the required shape, the change is mechanical (both producers already always spell the key or spread it conditionally) but it becomes a compile-breaking widening of a published input.

Write-surface enumeration (the triage's must-answer)

Is SqlHttpOutbox.insert the only door? Enumerated, not exemplified, at origin/main (ff37576):

  • Production constructors of an IHttpOutbox: exactly one — messaging-service-plugin.ts:310 (new SqlHttpOutbox(engine, ...), default object name). MemoryHttpOutbox is constructed only by tests.
  • Row-creating engine calls on the object: exactly one INSERT site, SqlHttpOutbox.insert, reached from enqueue() and recordUndeliverable() — both covered by this stamp. No bulkCreate/upsert/seed writes anywhere (grep -rn "insert(SYS_HTTP_DELIVERY|'sys_http_delivery'" over packages/ apps/: only the four test files that build their own rows).
  • The remaining writes (claim/ack/redeliver UPDATEs) create no rows and never touch the tenant column; userActions declares create/edit/delete false and the object is managedBy: 'engine-owned'.

Verification

  • New pins: packages/services/service-messaging/src/http-outbox-organization.test.ts (9 — SQL stamp verbatim / explicit-NULL normalization / parked-row stamp / read-back mapping; memory parity + all three arms of the redeliver predicate, ADR-0112 code asserted on the refusal); http-nodes.test.ts (+2 — tenant threaded verbatim; over-denial control: org-less run still enqueues, key absent, warns); auto-enqueuer.test.ts (+3 — per-record, org-less control, bulk path).
  • Targeted suites at head sha e2ecb71: service-messaging 7 files / 48 tests pass (incl. the tenant-audit and parked-record integration suites), service-automation 4 files / 34 pass, plugin-webhooks 3 files / 40 pass. typecheck green for service-messaging + plugin-webhooks (service-automation declares none); --listFiles confirms the new test file is inside the messaging typecheck.
  • Reverse verification (committed state, restore proven by blob hash vs HEAD): plugin-webhooks resolves @objectstack/service-messaging via dist/index.d.ts (tsc --listFiles); mutating the producer key to organizationIdX reddens typecheck with TS2561 naming organizationId as the fix — proving the rebuilt .d.ts carries the member and the literal is enforced against it. Restore verified byte-identical (worktree blob = HEAD blob 7f52692).
  • Gates at e2ecb71: all 14 path-derived families green except check:dual-build-cjs-loads, which is NOT MEASURED locally (PREREQUISITE NOT MET — it reads every package's dist and 41+ untouched packages are unbuilt here; CI's Build Core owns the full run). Declared narrowing: the gate's own population is every published package's built entry (quoted from its error output); this diff touches 3 packages and no build/exports config of any other, so untouched packages' verdicts cannot move; the 3 touched packages were rebuilt and their CJS entries require() clean (3/3 probed).

Generated by Claude Code


Generated by Claude Code

…ws (#13546)
Part of #13546 — the enqueue door never wrote the tenant column, so every
row landed in the SQL driver's (organization_id IS NULL) global-row arm and
the cross-organization wall on redeliver() (#10740) excluded nothing.
Mirrors the notification outbox's existing repair (EnqueueDeliveryInput):
- EnqueueHttpInput gains an optional organizationId (inherited by
UndeliverableHttpInput, so parked rows are stamped too); HttpDelivery
surfaces it on read-back.
- SqlHttpOutbox.insert writes organization_id: input.organizationId ?? null,
the same line SqlOutbox.enqueue writes.
- MemoryHttpOutbox stores the field and, now that its rows carry a tenant,
applies RedeliverOptions.tenantId in redeliver() with the driver's exact
semantics (other org invisible; org-less row global; tenant-less caller
unscoped).
- The flow http node (durable mode) threads AutomationContext.tenantId —
the notify node's #11303 source — and warns loudly when a run has none.
- The webhook auto-enqueuer stamps each delivery with its subscription's
own organization (sys_webhook.organization_id).
Forward-stamping only; existing NULL rows are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…string
check:doc-authoring red: operators cannot resolve #NNNN. The anchor stays
in the adjacent code comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-webhooks, @objectstack/service-automation, @objectstack/service-messaging, touching 13 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (4 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 214ce1a01bdf23e06de3ef9a877c3a36013531fd — the merge of head 479e6315071c3b8cef710d46da7e64b4d3a160c1 into base 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0, 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 214ce1a01bdf23e06de3ef9a877c3a36013531fd && git checkout 214ce1a01bdf23e06de3ef9a877c3a36013531fd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 479e6315071c3b8cef710d46da7e64b4d3a160c1 && git checkout -B drift-repro 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 && git merge --no-ff 479e6315071c3b8cef710d46da7e64b4d3a160c1
node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0

⚠️ 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… ledger
check-engine-double-contract --write: 1 row added (findOne, pinned: 1),
0 lost — additive ratchet only, per the gate's own instruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_http_delivery rows always land organization_id = NULL, so the deliberate cross-organization wall on redeliver() is reachable around for every row

3 participants

@os-steve@huangyiirene@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows by os-steve · Pull Request #13565 · objectstack-ai/objectstack · GitHub
Skip to content

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows - #13565

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id
Aug 31, 2026
Merged

fix(service-messaging): stamp organization_id on sys_http_delivery rows so the redeliver() cross-organization wall excludes other tenants' rows#13565
huangyiirene merged 4 commits into
mainfrom
claude/issue-13546-http-outbox-organization-id

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#13546

What breaks today

sys_http_delivery is tenant-scoped and redeliver() — its one request-reachable door (POST /api/v1/webhooks/redeliver, open to any authenticated user) — deliberately scopes by the caller's organization (#10740). But the enqueue door never stamped organization_id, and the SQL driver's tenant term is (organization_id = :tenantId OR organization_id IS NULL) — a deliberate global-row fail-open. So 100% of rows landed in the NULL arm: visible to, and replayable by, every organization on a walled deployment. Declared, not enforced; this PR restores the declared contract. The driver's tenant term is untouched — removing the IS NULL arm would hide true global rows from every tenant, a different defect.

The repair — mirror of the sibling that already received it

The notification outbox got exactly this fix (EnqueueDeliveryInput.organizationId, SqlOutbox.enqueue writing organization_id: input.organizationId ?? null). This PR gives the HTTP outbox the same shape, end to end:

Forward-stamping only. Existing NULL rows are untouched; their disposition is the separate decision the card names. No backfill, no driver changes, no schema changes (organization_id is kernel-provisioned on every object).

Optional vs required — argued, not assumed

The new member is optional (organizationId?: string). Weighed explicitly against the required-but-undefined-able shape this same file uses for RedeliverOptions.tenantId:

  • For optional: it mirrors the sibling notification outbox's EnqueueDeliveryInput.organizationId and EmitInput.organizationId — one convention across the two outboxes of one package (a second convention here would be this card's defect repeated). It is non-breaking for the published surface. And org-less enqueues are legitimate: a single-posture install and every stack before its first organization must keep enqueuing.
  • For required (organizationId: string | undefined): nothing tells a future producer it forgot one — the exact gap that produced this bug. That is a real cost. The mitigations shipped here: the producer population of this seam is enumerated (two, both repaired in this PR), the flow node warns loudly on an org-less multi-org run, and the driver's auditMissingTenant line still reports unscoped writes on multi-tenant boots. RedeliverOptions.tenantId went required because its callers are arbitrary request routes; this seam's callers are the two service-internal producers.

If the contract reviewer prefers the required shape, the change is mechanical (both producers already always spell the key or spread it conditionally) but it becomes a compile-breaking widening of a published input.

Write-surface enumeration (the triage's must-answer)

Is SqlHttpOutbox.insert the only door? Enumerated, not exemplified, at origin/main (ff37576):

  • Production constructors of an IHttpOutbox: exactly one — messaging-service-plugin.ts:310 (new SqlHttpOutbox(engine, ...), default object name). MemoryHttpOutbox is constructed only by tests.
  • Row-creating engine calls on the object: exactly one INSERT site, SqlHttpOutbox.insert, reached from enqueue() and recordUndeliverable() — both covered by this stamp. No bulkCreate/upsert/seed writes anywhere (grep -rn "insert(SYS_HTTP_DELIVERY|'sys_http_delivery'" over packages/ apps/: only the four test files that build their own rows).
  • The remaining writes (claim/ack/redeliver UPDATEs) create no rows and never touch the tenant column; userActions declares create/edit/delete false and the object is managedBy: 'engine-owned'.

Verification

  • New pins: packages/services/service-messaging/src/http-outbox-organization.test.ts (9 — SQL stamp verbatim / explicit-NULL normalization / parked-row stamp / read-back mapping; memory parity + all three arms of the redeliver predicate, ADR-0112 code asserted on the refusal); http-nodes.test.ts (+2 — tenant threaded verbatim; over-denial control: org-less run still enqueues, key absent, warns); auto-enqueuer.test.ts (+3 — per-record, org-less control, bulk path).
  • Targeted suites at head sha e2ecb71: service-messaging 7 files / 48 tests pass (incl. the tenant-audit and parked-record integration suites), service-automation 4 files / 34 pass, plugin-webhooks 3 files / 40 pass. typecheck green for service-messaging + plugin-webhooks (service-automation declares none); --listFiles confirms the new test file is inside the messaging typecheck.
  • Reverse verification (committed state, restore proven by blob hash vs HEAD): plugin-webhooks resolves @objectstack/service-messaging via dist/index.d.ts (tsc --listFiles); mutating the producer key to organizationIdX reddens typecheck with TS2561 naming organizationId as the fix — proving the rebuilt .d.ts carries the member and the literal is enforced against it. Restore verified byte-identical (worktree blob = HEAD blob 7f52692).
  • Gates at e2ecb71: all 14 path-derived families green except check:dual-build-cjs-loads, which is NOT MEASURED locally (PREREQUISITE NOT MET — it reads every package's dist and 41+ untouched packages are unbuilt here; CI's Build Core owns the full run). Declared narrowing: the gate's own population is every published package's built entry (quoted from its error output); this diff touches 3 packages and no build/exports config of any other, so untouched packages' verdicts cannot move; the 3 touched packages were rebuilt and their CJS entries require() clean (3/3 probed).

Generated by Claude Code


Generated by Claude Code

…ws (#13546)
Part of #13546 — the enqueue door never wrote the tenant column, so every
row landed in the SQL driver's (organization_id IS NULL) global-row arm and
the cross-organization wall on redeliver() (#10740) excluded nothing.
Mirrors the notification outbox's existing repair (EnqueueDeliveryInput):
- EnqueueHttpInput gains an optional organizationId (inherited by
UndeliverableHttpInput, so parked rows are stamped too); HttpDelivery
surfaces it on read-back.
- SqlHttpOutbox.insert writes organization_id: input.organizationId ?? null,
the same line SqlOutbox.enqueue writes.
- MemoryHttpOutbox stores the field and, now that its rows carry a tenant,
applies RedeliverOptions.tenantId in redeliver() with the driver's exact
semantics (other org invisible; org-less row global; tenant-less caller
unscoped).
- The flow http node (durable mode) threads AutomationContext.tenantId —
the notify node's #11303 source — and warns loudly when a run has none.
- The webhook auto-enqueuer stamps each delivery with its subscription's
own organization (sys_webhook.organization_id).
Forward-stamping only; existing NULL rows are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…string
check:doc-authoring red: operators cannot resolve #NNNN. The anchor stays
in the adjacent code comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actionsBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-webhooks, @objectstack/service-automation, @objectstack/service-messaging, touching 13 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (4 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 214ce1a01bdf23e06de3ef9a877c3a36013531fd — the merge of head 479e6315071c3b8cef710d46da7e64b4d3a160c1 into base 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0, 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 214ce1a01bdf23e06de3ef9a877c3a36013531fd && git checkout 214ce1a01bdf23e06de3ef9a877c3a36013531fd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 479e6315071c3b8cef710d46da7e64b4d3a160c1 && git checkout -B drift-repro 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 && git merge --no-ff 479e6315071c3b8cef710d46da7e64b4d3a160c1
node scripts/docs-audit/affected-docs.mjs --json 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0

⚠️ 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 51ecb2fdd2035a8924603b497a6b7a2ee8277fb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… ledger
check-engine-double-contract --write: 1 row added (findOne, pinned: 1),
0 lost — additive ratchet only, per the gate's own instruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_http_delivery rows always land organization_id = NULL, so the deliberate cross-organization wall on redeliver() is reachable around for every row

3 participants

@os-steve@huangyiirene@claude