Skip to content

[F167] No end-to-end proof that the F225 callback binds its approval origin (systemOriginExemption is a declaration, not a verification) #1350

Description

@TERRYYYC

Status: core coverage landed 2026-08-13 — this issue is narrowed, not closed

PR #1349 at 2b170e2b78f4528b6e294241880830a61036b2fe now carries the coverage this
issue was opened to defer. Both reviewers asked for it independently (maintainer
direction gate 2; @codex-luna's Gen-1 P2), so it was written rather than tracked:

  • approval-hub/scheduler-origin-callback-integration.test.js — real route + real
    InvocationRegistry record + real scheduler wake row, for F225 / F128 / F193 / F231 /
    F260 accept and F221 (forbidden) reject on the identical path.
  • propose-session-handoff-route.test.js — F225 depth, including scope item 2:
    a body naming another thread / message / owner cannot move the persisted originRef.

Sensitivity was measured: flipping F225 to forbidden reddens exactly the three accept
cases and leaves both negatives green.

Scope items 1, 2 and 3 below are therefore closed. What remains open is the
"Suggested extension", and one item in it is not a nice-to-have:

F193 and F260 rest on a creation-site audit performed by the reviewer and never
independently re-walked by the PR author.
The new tests prove those two routes bind the
origin as observed through the route; they do not supply a second pair of eyes on the
adapters themselves. That is a provenance gap, not a coverage gap, and closing it means
someone other than @codex-luna walking deriveCallbackOriginRef and deriveEntityOriginRef.

Keep this open until that happens. The catalog comments at those two entries say the same
thing, so the two records agree.

Summary

The systemOriginExemption catalog gate introduced for #1348 (PR #1349) has no end-to-end coverage proving the caller-side attestation it relies on is real. This issue holds durable custody of that gap so it cannot be closed by the catalog declaration alone.

Opened at maintainer direction on PR #1349 (2026-08-12T14:57Z), direction gate 2: "Either add the authenticated F225 callback → ApprovalIngress integration test in this PR, or open and link a tracked follow-up issue. The integration test is deferrable, but the caller-side attestation proof must have durable custody." This issue is that custody.

Why the existing tests are not enough

PR #1349 adds five ingress-level unit tests, and they do cover the decision matrix at validateOrigin:

assertion what it proves
scheduler origin accepted for a server_attested producer the gate opens where intended
same origin rejected for a forbidden producer the gate is per producer, not global
F128 (transitive binding) accepted the transitive shape is in scope deliberately
another human owner still rejected cross-tenant isolation intact
system userId + real cat catId still rejected the exemption is unreachable by a cat wearing a system ID

All five construct the draft directly. They prove the ingress decides correctly given a producerId and an origin row. None of them proves the premise the whole design rests on:

the F225 producer derives originRef.threadId, originRef.messageId and ownerUserId from an authenticated InvocationRecord, and a request body cannot rewrite them.

That premise is currently asserted by a comment and a catalog value, both written by hand. systemOriginExemption: 'server_attested' is a declaration, and required only stops an omission — it cannot stop a wrong declaration. A future refactor that let the request body supply sourceMessageId on the session-handoff route would keep every one of the five tests green while silently converting the exemption into the cross-tenant hole it was scoped to avoid.

That is the shape of gap this issue exists to close: there is no red test that the strongest claim in the change is true.

Scope

Add coverage that starts at the authenticated callback boundary, not at the ingress:

  1. Positive path. An authenticated propose_session_handoff callback on a scheduler-woken invocation (persisted wake row userId='scheduler', catId=null, in the owner thread) produces an approval card. This is the exact scenario of [F167] Scheduler wake origin blocks session handoff approval #1348 and today has no end-to-end assertion anywhere.
  2. Binding is not body-controlled. The same callback with a request body attempting to name a different threadId / messageId / owner must not be able to move the origin. The assertion should be on the resulting originRef (it equals the record-derived values), not merely on a status code — a route that ignores the field and one that validates it are indistinguishable from the status alone.
  3. Non-attested producer. The equivalent callback for a forbidden producer still fails closed at the ingress when its origin is system-authored.

Item 2 is the one that carries the weight. Items 1 and 3 are regression anchors.

Suggested extension

Once the F225 route has this coverage, the other server_attested entries deserve the same treatment, and they are not equally proven today. The catalog records the difference honestly:

  • F231 — DIRECT and strictest: a body-supplied sourceMessageId is rejected unless it equals the record-derived value.
  • F128 — TRANSITIVE: the proposal row is written entirely from the record, then originRef is built from that row.
  • F193 / F260 — same transitive shape, but their creation-site audit was performed by the reviewer and not independently re-walked by the PR author.

F193 and F260 are therefore the weakest links by provenance, not by design. Route-level coverage for them is worth more than more ingress-level cases.

Acceptance criteria

  • An integration-level test exercises the authenticated F225 callback path end to end with a scheduler-authored origin row and asserts the proposal is created.
  • A test asserts the resulting originRef is record-derived, by attempting to override thread/message/owner through the request body and asserting the persisted origin is unchanged.
  • A test asserts a forbidden producer still fails closed on the same input.
  • The tests fail if systemOriginExemption were flipped to server_attested on a producer whose route does not bind — i.e. they test the binding, not the declaration.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestneeds-infoWaiting for additional information from reportertriagedMaintainer reviewed, replied, and made an initial triage decision

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions