Skip to content

item 7: the webhook approval provider - #13

Merged
arpanghoshal merged 2 commits into
mainfrom
item-7-webhook
Sep 3, 2026
Merged

arpanghoshal merged 2 commits into
mainfrom
item-7-webhook

Conversation

@arpanghoshal

Copy link
Copy Markdown
Member

Build-list item 7. SPEC-v0.2 §7, acceptance tests T27 and T28.

Outbound is core, not an extra: it needs neither an HTTP server nor an SDK, just one signed POST over stdlib urllib.request. The inbound endpoint is the gateway's, because that is the one server this release ships.

The signature is over the exact bytes sent

CTRLRun-Signature: t=<unix seconds>,v1=<hex>, HMAC-SHA256 over f"{t}.{body}". Two encoders that agree on meaning and differ on whitespace would produce a MAC the receiver cannot verify, and a receiver that re-serialized the payload to check would be verifying its own guess rather than what arrived.

Inbound, every condition is checked before anything is written, and the refusal message has the same shape for all of them: telling a caller which condition it missed is telling it what to try next. A replayed answer is idempotent only where it asks for the answer the record already carries — a grant replayed onto a denied or consumed record is a different question, and is refused. Single use stays where v0.1 §4.2 A2 put it, at consumption.

An undelivered notification is not an approval. Every attempt failing leaves the request pending, ApprovalRequired raised with a request_id that ctrlrun approve can still answer, and a warning logged without the secret or the payload — the payload carries the arguments, which are the customer identifiers and amounts.

One thing that changed outside §7

urllib.request is now imported inside the methods that use it. It drags in ssl, and §1.1's rule — a core install pays only for what it uses — reads the same against the stdlib as against the extras.

The examples' no-network harness caught this, which is the nice part: it had replaced socket.socket with a function, and ssl does class SSLSocket(socket). The harness now replaces the type with one that refuses to connect, which is what a cut cable actually looks like. tests/test_packaging.py pins ssl out of import ctrlrun.

Mutation table — 20 mutations, all red

# Group Result
M1–M4 the MAC's construction, verification, compare_digest, the replay window red
M5–M8 path/body ids, action hash, the request existing, the decision vocabulary red
M9–M11 idempotent only for the same answer; a store refusal is a 400; a refusal changes nothing red
M12–M15 record before announce; failures never raise into the kernel; bounded retries; HTTPError is a failure red
M16–M20 redirects, the http:// rule, the secret's length and emptiness, respond_to red

Eight started green. Two were harness faults of familiar kinds. The rest were real, and two are worth your time:

if response.status >= 400 was dead code. urlopen raises HTTPError for anything at or above 400, and HTTPError is a URLError, which _deliver already treats as a failed delivery. The branch could never run.

The redirect test used a 307 — the one code urllib refuses on a POST by itself. So the no-redirect handler, which is the whole defence against delivering a signed payload somewhere the operator did not name, was never exercised. urllib follows 301, 302 and 303, converting the POST to a GET; and the test's second server only recorded POSTs, so even a followed redirect was invisible to the assertion. Both fixed: parametrized over the three codes urllib actually follows, and the second server records GETs.

That second one is a shape worth naming alongside the subsumed-guard pattern: a negative test proves nothing unless the thing it forbids would otherwise happen. I asserted "the redirect was not followed" against a case where nothing would have followed it anyway, and against a recorder that could not have seen it if it had.

Also fixed: the path/body id test passed for the wrong reason (with only the path wrong, the lookup finds nothing), and neither an unknown request id nor a nonsense decision had any test.

Verification

1207 tests pass (1162 → 1207), mypy --strict src/, ruff check and ruff format --check clean. import ctrlrun touches neither ssl, httpx nor ctrlrun.gateway.

Remaining: item 8 (OTel sink + ACS note, T29 and T30's named test), then item 9 releases.

SPEC-v0.2 §7, acceptance tests T27 and T28. Outbound is core because it needs
neither an HTTP server nor an SDK: one signed POST over stdlib urllib.request.
The inbound endpoint is the gateway's, because that is the one server this
release ships.

The signature is over the exact bytes sent. Two encoders that agree on meaning
and differ on whitespace would produce a MAC the receiver cannot verify, and a
receiver that re-serialized the payload to check would be verifying its own
guess rather than what arrived.

Every inbound condition is checked before anything is written, and the refusal
message has the same shape for all of them: telling a caller which condition it
missed is telling it what to try next. A replayed answer is idempotent only
where it asks for the answer the record already carries — a grant replayed onto
a denied or consumed record is a different question, and is refused. Single use
stays where v0.1 §4.2 A2 put it, at consumption.

An undelivered notification is not an approval. If every attempt fails the
request stays pending, ApprovalRequired is raised with a request_id that
`ctrlrun approve` can still answer, and the failure is logged without the
secret or the payload — the payload carries the arguments, which are the
customer identifiers and amounts.

Redirects are never followed: one to another host would deliver the signed
payload, and the action inside it, somewhere the operator did not name. An
http:// url is refused unless it is loopback and meant.

urllib.request is imported inside the methods that use it, not at module
scope. It drags in `ssl`, and §1.1's rule — a core install pays only for what
it uses — reads the same against the stdlib as against the extras. The
examples' no-network harness caught this: it had replaced socket.socket with a
function, which `ssl` cannot subclass. The harness now replaces the type with
one that refuses to connect, which is what a cut cable actually looks like.
…branch

Mutation-testing §7 found more than the usual crop.

Two were untested rather than subsumed: no test sent a request id the store
had never seen, and none sent a decision that was neither grant nor deny. Both
now exist.

The path/body request-id comparison had a test that passed for the wrong
reason: with only the path wrong, the lookup finds nothing and the refusal is
right by accident. Parametrized over both directions now — with only the body
wrong, the lookup finds the *real* record and the comparison is the only thing
between it and a grant.

`if response.status >= 400` was dead: urlopen raises HTTPError for anything at
or above 400, and HTTPError is a URLError, which _deliver already treats as a
failed delivery. Removed.

The redirect test was the interesting one. It used a 307 — the one code urllib
refuses on a POST by itself — so the no-redirect handler was never exercised.
urllib follows 301, 302 and 303, converting the POST to a GET, and the test's
second server only recorded POSTs, so even a followed redirect was invisible
to it. Parametrized over the three codes urllib actually follows, and the
second server records GETs.

The empty-secret check is subsumed by the length check and stays for its
message: "empty" points at the configuration, "too short" at the value. The
test asserts which one it got.
@arpanghoshal
arpanghoshal merged commit f9b9d18 into main Sep 3, 2026
3 checks passed
@arpanghoshal
arpanghoshal deleted the item-7-webhook branch September 3, 2026 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant