fix(TC-1132): harden webhook, quota, and tunnel egress

[samgbafa]

TC-1132: webhook, quota and tunnel egress hardening

Addresses TC-1132, TC-1080 Parts C–D. Part B remains a documented REQUIRED future-enable gate; network DID methods stay disabled by default. No hook development flag or did:web fetcher was introduced.

Current candidate

2a3466dd31ae7548f892008b4e7cbfabeca6cfae supersedes rejected f30a352. PR remains draft until new-head CI and both independent re-reviews clear. No merge/deploy authorization is assumed.

Corrections to independent review blockers

• Refresh now holds exclusive deny-state access through bounded endpoint DNS and commits state/health only after completion. Valid deliveries wait for an unchanged successful refresh rather than observing a false-unhealthy gate and becoming dead-lettered. Cancelling a refresh leaves the previous state intact; stale/failed state remains fail-closed.
• Callback validation preserves typed errors. Temporary DNS/policy failure and sender timeout use the existing bounded retry schedule; invalid/forbidden callbacks remain terminal. Registration HTTP contracts and post-auth lookup order are preserved.
• Removed the raw reqwest client API. egress::post(policy, VettedDestination, timeout) returns a sealed EgressRequest with header/body/send only. The canonical URL is privately owned and cannot be retargeted or used to override proxy/redirect/TLS policy.
• Send rechecks freshness/health and all vetted addresses under the exact read lease held through connection/request. Literal hosts receive the same send-time enforcement as hostnames. The fixed resolver uses only the lease-bound permitted subset and never reacquires policy state or performs a second DNS lookup.
• One total send budget covers lease/freshness waits and HTTP send; reqwest receives only the remaining budget. Lock waits and DNS are bounded to one second, connect remains capped at two seconds. Cancellation releases read leases.

Preserved scope

• Precise special-use IPv4/IPv6 and embedded-v4 checks; corrected public controls; conservative ambiguous local-use NAT64 handling.
• Periodically refreshed DB/quota/allowlist/explicit-S3 deny-set, including IP aliases and all ports.
• HTTPS-only, no redirects/proxy environment, original body/header/HMAC bytes, URL<=2048 raw/canonical bytes, secret<=256 bytes, hour backoff for attempts>=6 and max-attempt ceiling10.
• Tested Part D commit d874358f cherry-picked as9f702dd: quota one-segment encoding/no redirects; tunnel no redirects/30s cumulative timeout and existing response/error framing. D files remain unchanged by blocker correction.

Verification

• Independent f30a352 red evidence accepted from both exact-head REQUEST_CHANGES reviews; original failures were not rerun merely to confirm them.
• Corrected local consumer regressions cover successful-refresh delivery, failed-policy retry/recovery, literal refusal after revalidation becomes Forbidden, TLS literal identity, mixed-subset filtering, cancellation both waiting and holding a lease, cancellation of refresh, and total deadline including refresh wait.
• cargo test -p tinycloud-node --lib tc1132_ -- --nocapture: 28 passed.
• cargo clippy -p tinycloud-node --all-targets -- -D warnings and cargo fmt --all -- --check: passed. Existing dependency future-incompatibility notice only: proc-macro-error2 v2.0.1.
• Disposable cargo test -p tinycloud-node --lib tc1132_corrected_actual_tcp_smoke -- --ignored --nocapture: real TCP service, four signed registrations200, embedded metadata400, healthy unchanged refresh defers four events then all four HTTPS/body/HMAC deliveries succeed at attempt1; DNS10=4registration+4delivery+2internal; proxy TCP0. Runner removed.
• Exact committed-head full suite in a disposable network namespace with local PostgreSQL14: TINYCLOUD_TEST_POSTGRES_URL=postgres://tinycloud@127.0.0.1:54382/postgres TINYCLOUD_TEST_BLACKHOLE=1 CARGO_NET_OFFLINE=true cargo test -p tinycloud-node -- --test-threads=1 — 573 passed, 0 failed, 1 existing manual benchmark ignored, including490 library tests, all4 real packet-drop blackhole tests and14 tunnel-service tests. Namespace/database/runner cleanup completed; no host firewall change.
• All three exact-head workflows completed/success at2a3466dd31ae7548f892008b4e7cbfabeca6cfae: Rust/workspace, Docker Container, Release. Deployment/release publication jobs were skipped.
• Both independent exact-head COMMENTED APPROVE verdicts received: egress reviewer and fresh consumer reviewer. PR marked ready for review with base main and unchanged head2a3466dd31ae7548f892008b4e7cbfabeca6cfae; no merge/deployment performed or authorized by this readiness change.

Risks / non-goals

• Failed internal-endpoint resolution closes egress until recovery; bounded retries can exhaust if an outage persists. Truly forbidden destinations dead-letter.
• Internal service IP aliases/shared addresses are denied across ports; ambiguous local-use NAT64 may refuse a shorter-prefix public translation when another valid interpretation is forbidden.
• A send lease can defer refresh until the configured total deadline; queued state waits fail closed within one second.
• Tunnel upstream operations exceeding30s time out. Sequential dispatch, atomic subscription caps and full Part B remain out of scope.
• No production, operator-secret, Docker, other-worktree, merge or deploy changes.

[samgbafa]

REQUEST_CHANGES

Independent code-correctness review of f30a352 only. Parts C (9a1712e), D (9f702dd), and the final NAT64 boundary commit were reviewed against TC-1132 acceptance and the original TC-1080 plan. Two reproduced correctness blockers remain despite green exact-head CI. This is a COMMENTED review because the GitHub identity is also the PR author; it is not approval.

1. [P1] Successful endpoint refresh permanently loses otherwise valid deliveries

Locations: egress.rs:130–138, egress.rs:165–181, and webhook_dispatcher.rs:140–155.

refresh_locked() marks the previously healthy, still-fresh deny-set unhealthy before awaiting DNS. A concurrent validation sees a fresh timestamp, does not await refresh_lock, and immediately returns Unavailable. The dispatcher erases the error category and terminally dead-letters that delivery. No stale state, DNS failure, destination change, or invalid callback is necessary. The liftoff refresh task repeats this exposure every ten minutes.

Deterministic consumer repro: start with db.test -> 192.2.1.1, register an authorized literal public callback, queue an event, start a semaphore-gated refresh returning the same DB address, and dispatch while that successful refresh is pending. Release DNS and observe that the callback is permitted again, but the event remains:

successful healthy refresh: delivery status=dead_letter, attempts=1, next=None,
error=Some("callback URL rejected: callbackUrl rejected: internal endpoint deny-set is unavailable")

independent_healthy_refresh_does_not_dead_letter_authorized_delivery fails its assertion against terminal data loss. The synthetic public addresses are only policy inputs; no public socket is opened.

Required correction: distinguish an in-progress refresh from a failed/unavailable refresh. Await the shared bounded refresh before deciding, or otherwise preserve the event for an unavailable-policy condition. Keep actual forbidden destinations fail-closed. A routine successful refresh must not permanently discard an authorized event.

2. [P2] Literal callbacks bypass the connect-time deny-set recheck

Locations: egress.rs:219–234, egress.rs:380–405, and egress.rs:425–442.

The latest deny-set/health check lives exclusively in PolicyResolver. Reqwest skips that resolver for an IP-literal URL, so literals receive only the earlier resolve_url() check. The accepted public-literal callback path can therefore connect using obsolete permission if a refresh interleaves between validation and connection. The hostname path has a later enforcement checkpoint that the literal path lacks.

Deterministic local-socket repro: under the existing test-only loopback seam, vet https://127.0.0.1:<own-port>/hook; refresh db.test from a synthetic permitted address to 127.0.0.1; verify a new resolve_url() now returns Forbidden; send the previously vetted destination through egress::client. The listener accepts TCP:

literal after successful deny refresh: connected=true

independent_literal_refresh_must_prevent_tcp_connection fails. The equivalent hostname control passes with zero connections and no second DNS. The listener immediately closes accepted connections; this proves a connection-policy bypass, not successful TLS/HMAC payload disclosure. The loopback exception only permits initial vetting: the internal deny-set still overrides it, as the intervening refusal proves. This models the refresh interleaving for production-permitted public literals without contacting a public target.

Required correction: give literal destinations the same final deny-set/health enforcement as hostname destinations. Do not rely exclusively on a DNS resolver for a URL class that bypasses DNS. Preserve the accepted public-literal behavior or explicitly reconcile any change with the acceptance tests.

Additional API gap, not a separate current-route SSRF claim

The exported client() returns an unrestricted reqwest::Client. With the production-equivalent policy (allow_loopback=false), vetting synthetic public.test and then posting the returned client to an unvetted https://127.0.0.1:<own-port>/unvetted still opens TCP. independent_raw_client_cannot_retarget_to_forbidden_literal fails. The current dispatcher posts the validated URL; I am not claiming it accepts such a retarget from a user. The destination-bound guarantee is not enforced by this reusable API, and a future Part B caller must not mistake it for one.

Independent verification actually exercised

All runtime work used an exact-commit disposable /tmp snapshot, offline Cargo, a clean explicitly constructed environment, SQLite memory databases, synthetic fixture keys, injected DNS, and owned 127.0.0.1 listeners. Only test additions in that disposable snapshot were made; the detached review worktree and PR source were not edited. Repro files and the instrumented snapshot are retained at /tmp/tc1132-review-f30a352-pM0c4Fmi.

Commands below run from that snapshot with CARGO_NET_OFFLINE=true, CARGO_TARGET_DIR=$snapshot/target, and TMPDIR=$snapshot:

• cargo test --offline -p tinycloud-node --lib tc1132_ -- --nocapture --test-threads=1: 19 passed on the uninstrumented exact snapshot. Includes registration authorization/byte limits, rebinding, special-use boundaries, DNS/connect deadlines, proxy/redirect refusal, HMAC/body, refresh failure/recovery, and backoff.
• cargo test --offline -p tinycloud-node --lib independent_ -- --nocapture --test-threads=1: 7 passed, 3 failed. Eight new reviewer probes plus two existing tests matching the filter. The three failures are the two blockers and the explicitly qualified API gap above.
• The independent real TCP Rocket hook-route smoke passed: four simultaneous authorized registrations returned 200; embedded metadata returned 400; the dispatcher delivered four exact TLS bodies with independently checked HMAC, content type, and delivery/event/subscription headers. DNS calls were exactly eight (registration/delivery once each); the owned HTTPS/ALL proxy listener received zero connections.
• Independent TLS probes refused an untrusted certificate and a trusted certificate with the wrong hostname before any HTTP payload; the matching trusted hostname delivered the exact body. Alias probes covered decimal/octal/hex/shortened IPv4, mapped/compatible IPv6, Teredo/site-local/6to4, all four local NAT64 layouts, and corrected public controls. Thirty-two simultaneous stale checks shared one refresh.
• cargo test --offline -p tinycloud-node --lib independent_ten_real_tls_failures_stop_at_ceiling_with_bounded_backoff -- --nocapture --test-threads=1: 1 passed. Ten actual TLS 503 replies; 30/120/600/3600-second backoffs; terminal attempt 10; no eleventh send.
• cargo test --offline -p tinycloud-node --lib independent_every_configured_service_denies_literal_and_embedded_aliases -- --nocapture --test-threads=1: 1 passed. Database, quota, allowlist, and explicit S3 IPs refused on alternate ports and through mapped/NAT64 aliases; unrelated public control permitted; no service opened.
• cargo test --offline -p tinycloud-node --test independent_quota --test tunnel_service -- --nocapture --test-threads=1: 1 independent quota test + all 14 tunnel tests passed. Quota 301/302/303/307/308 never reached the redirect listener or forwarded the fixture bearer; the encoded SpaceId remained one segment and fallback was preserved. Tunnel redirect framing, stalled headers/body deadlines, and healthy recovery on the same WebSocket passed.
• cargo test --offline -p tinycloud-node --test did_resolution_routes -- --nocapture --test-threads=1: 4 passed using tinycloud::app_with_control. /invoke, /delegate, and /revoke refused network DID issuers without loopback connections; /info reported network_methods=false and count-only refusal evidence.

Read all 15 changed paths: CHANGELOG.md, README.md, docs/delegation-flow.md, docs/specs/node-control-plane-v1.md, src/config.rs, src/egress.rs, src/egress/test_support.rs, src/egress/tests.rs, src/lib.rs, src/quota.rs, src/routes/hooks.rs, src/routes/mod.rs, src/tunnel/connection.rs, src/webhook_dispatcher.rs, and tests/tunnel_service.rs (Rust paths under tinycloud-node-server/). The Part B documentation makes the full node-owned did:web fetcher/limits/refusal tests mandatory before future enablement; this PR does not implement that fetcher. Sequential delivery and the pre-existing non-atomic subscription cap remain explicitly deferred, not new blockers here.

Limits: no independent full PostgreSQL/packet-blackhole suite or SYN-drop test was run; the owner's 564-pass report is not my evidence. Local TLS connect-stall and tunnel deadlines were exercised. No children, Docker, host firewall changes, production access, external test destinations, operator secrets/environment inspection, push, merge, or deployment.

Exact-head CI gate — independently verified green

For each run, gh run view <id> --repo TinyCloudLabs/tinycloud-node --json headSha,status,conclusion,url returned headSha=f30a352afaa3f10374ae84a8df3e316f0fac4bdc, status=completed, and conclusion=success:

• Rust / workspace build, fmt, clippy, PostgreSQL-backed tests
• Docker Container
• Release

Green CI clears the publication gate, not the reproduced correctness failures. REQUEST_CHANGES for this exact candidate.

[samgbafa]

[P1] A successful refresh of a still-fresh deny-set creates an Unavailable window; a concurrent authorized delivery takes webhook_dispatcher.rs:140–155 and permanently dead-letters. Deterministic semaphore-gated DNS repro: unchanged DB IP, registration200, refresh ultimately succeeds, event dead_letter at attempt1 with next=None. Await the in-progress bounded refresh (or preserve unavailable-policy events) before treating the destination as terminally invalid; keep genuinely forbidden destinations fail-closed. Full evidence in the review.

[samgbafa]

[P2] This final deny-set/health enforcement is DNS-only. Reqwest skips PolicyResolver for literal callback URLs. A local socket repro vets a literal, successfully refreshes its IP into the internal deny-set, verifies new resolve_url() refuses, then sends the earlier vetted destination: TCP is accepted. Equivalent hostname control opens no socket. Give literals the same final enforcement checkpoint; this is a connect-policy bypass, not a claim that TLS/HMAC payload delivery succeeded. Full evidence and the separately qualified raw-Client API gap are in the review.

[samgbafa]

Independent consumer-contract / failure-path review — REQUEST_CHANGES

Reviewed f30a352afaa3f10374ae84a8df3e316f0fac4bdc, base main, all 15 changed paths, Linear TC-1132 acceptance and TC-1080 PLAN Parts B–D. This is a COMMENTED review because the authenticated GitHub identity is the PR author; the engineering verdict is REQUEST_CHANGES, not approval.

Blocking findings

1. [P2] A successful scheduled deny-set refresh permanently discards valid webhook deliveries.

egress.rs:130–158 marks the shared set unhealthy before awaiting DNS, including when the old set is fresh and all answers are unchanged. Concurrent URL validation returns Unavailable; webhook_dispatcher.rs:140–155 converts that temporary state into a terminal failure.

Independently exercised sequence: initialize a healthy internal deny-set; confirm https://198.51.0.1/callback permitted; queue an authorized delivery; start a refresh with an injected resolver gated by Notify; dispatch while DNS is outstanding; release the resolver with the same successful answer; confirm the destination is permitted again. Observed row:

status=dead_letter, attempts=1, next_attempt_at=None
last_error="callback URL rejected: callbackUrl rejected: internal endpoint deny-set is unavailable"

No callback connection was attempted. The safety assertion that a healthy refresh must not permanently discard the delivery failed. This is ordinary background-refresh overlap, not the documented case of a newly forbidden destination. Any configured network DB/quota/allowlist/S3 host can open this window every ten minutes. Keep egress fail-closed, but do not terminally classify an in-progress/unavailable policy refresh as a forbidden callback; wait for the bounded refresh or preserve retryable delivery state. Add a deterministic concurrent successful-refresh regression.

2. [P1] Literal-IP requests bypass the connect-time internal deny-set recheck.

egress.rs:425–442 returns a raw reqwest::Client and installs PolicyResolver as its only connect-time policy enforcement. Reqwest skips that resolver for IP-literal URLs. The literal check at egress.rs:221–234 is earlier; it does not protect a previously vetted destination after the deny-set changes.

Independent loopback HTTPS reproduction: vet a literal callback; refresh an internal-host answer so that the callback IP becomes denied; confirm fresh resolve_url returns Forbidden; call egress::client(policy, previously_vetted, timeout).post(the_same_literal_url).send(). Observed:

new validation=Forbidden; send status=Ok(200); callback_received=true

The test-only loopback class exception and fixture certificate enabled safe local transport; the internal deny-set itself was not bypassed by any test flag. No external/internal-service connection was made. The assertion that the stale-vetted literal must be refused failed. This establishes a literal-only policy gap across refresh, not an arbitrary-host retargeting exploit. Enforce current destination policy on the literal request path as well as DNS names, and bind the request to the vetted URL instead of treating a raw reqwest client as a policy boundary. Add a literal counterpart to the existing hostname connect-time refresh test.

Independent exercised evidence

Own throwaway Rust probes compiled the exact checked-out node modules, adding only a test module and fixture routes outside the checkout. No PR source changes. Five independent probes passed; the two blocking safety assertions above failed. Initial fixture assumptions (legacy plaintext encryption format and normalized hook scope) were corrected before the passing fixture runs; those were not PR findings.

• Actual TCP tinycloud::app: signed /delegate 200 and /invoke 200; loopback billing observed one encoded quota path segment decoding to the original SpaceId containing %2F, plus the fixture bearer; public 192.2.1.1 webhook registration 200; embedded NAT64 metadata callback registration 400; /info reported network_methods=false.
• Actual WS tunnel targeting that app: 307 status, Location, and body preserved. An upstream delayed headers 20 seconds, then sent body bytes every 2 seconds: cumulative timeout observed at 30.045690996 seconds, via the existing request-local body-read error, with no partial response for that request. Concurrent /info and the next /info succeeded over the same authenticated WebSocket.
• Quota: constructed SpaceId with /, ?, #, %, .., and UTF-8; encoded segment decoded exactly to the original key; cached remote result and admin override used the original key. 307/308 used fallback and made zero connections to the redirect listener; fixture bearer was present on the configured billing request.
• HTTPS egress: mixed injected DNS discarded metadata and deduplicated the permitted loopback surrogate. Changing DNS after vetting did not cause a second lookup at connect; the next attempt refused the private answer. 307 was not followed; redirect and configured HTTPS proxy listeners each observed zero connections.
• Dispatcher: one corrupted secret did not stop other deliveries; exact UTF-8 payload bytes, HMAC and delivery/event/subscription/content-type headers verified; attempt 6 scheduled at least one hour; attempt 10 dead-lettered; rebinding to private IP refused without callback.
• IP boundaries: independent ambiguous local-use NAT64, embedded metadata, Teredo/site-local and public-range edge matrix passed.
• Additional exact-head existing focused tests run here: cargo test -p tinycloud-node --lib tc1132_ --offline -- --nocapture --test-threads=1 — 19 passed; cargo test -p tinycloud-node --test did_resolution_routes --offline -- --nocapture --test-threads=1 — 4 passed. Their success does not cover the two newly exercised blockers.

Local reproduction resources for Miller: /tmp/tc292-consumer-01a13ba4/consumer_tests.rs, build-review.py, compiler.json, and repo/tinycloud-node-server/src/review_root.rs. Build with python3 /tmp/tc292-consumer-01a13ba4/build-review.py from the isolated checkout. Run /tmp/tc292-consumer-01a13ba4/tinycloud-efe4f8d807b9bfc8 consumer_review:: --nocapture --test-threads=1; the two tests named independent_successful_refresh_must_not_permanently_drop_delivery and independent_literal_ip_must_recheck_changed_denyset_before_connect are expected to fail on this SHA. All fixture keys/service bearers are disposable constants.

Nonblocking observations / test limits

Quota encoding/cache identity, redirect framing, cumulative tunnel timeout and same-socket health were satisfactory in the exercised cases. Documentation correctly makes full Part B mandatory before any network-DID re-enable and does not claim to implement the fetcher. This remains an operator/developer gate: the pre-existing network_methods=true option still reaches the unhardened SSI path; do not use it as rollback.

I did not independently repeat the owner's PG14/packet-blackhole/full 564-test run or local fmt/clippy; those remain owner/CI evidence. My transports were local IPv4 TCP/TLS/WS with injected DNS; IPv6/NAT64 were policy tests, not live IPv6 translation. No production, host firewall, Docker, credential/env-file, other-agent worktree, merge/deploy/push access. No child agents. No runtime claim for the unimplemented Part B fetcher.

Exact-head CI gate — independently checked, green

All three run metadata records had head_sha=f30a352afaa3f10374ae84a8df3e316f0fac4bdc, status=completed, conclusion=success before this review:

• Rust
• Docker Container
• Release

CI clears the submission gate, not these blockers. Withhold approval of this exact SHA.

[samgbafa]

[P2] Blocking: successful scheduled refresh can permanently drop valid deliveries. Independently gate the internal DNS lookup here after a healthy initial refresh, dispatch an authorized pending webhook while the same-answer refresh is outstanding, then release DNS. Observed dead_letter, attempts=1, next_attempt_at=None, error="callback URL rejected: callbackUrl rejected: internal endpoint deny-set is unavailable"; destination permitted before and after. webhook_dispatcher.rs:140–155 terminally classifies this temporary state. Keep fail-closed egress, but wait for bounded refresh or preserve retryable delivery state; add concurrent successful-refresh coverage.

[samgbafa]

[P1] Blocking: reqwest skips this resolver for IP-literal request URLs. Independent local HTTPS test vets a literal callback, refreshes the internal host to that IP, confirms new resolve_url is Forbidden, then sends with client(policy, old_vetted).post(the_same_literal). Observed 200 and callback_received=true. The test-only loopback allowance does not override internal denial. Enforce current policy on literal sends too; the raw Client is not a connect-time policy boundary. Existing hostname refresh coverage does not exercise this branch. Full sequence/evidence and limits in review body.

[samgbafa]

APPROVE

Independent re-review of 2a3466d only, comparing its six-path correctness delta against the previously reviewed f30a352. The earlier REQUEST_CHANGES review remains the record for that older candidate; this review assesses the new SHA.

Prior blockers and API gap: independently closed

• Successful refresh no longer loses valid queued events. egress.rs:139–173 retains exclusive state access through bounded DNS and changes the completed state only after the result is known. The original semaphore-gated queued-webhook probe now observes pending, attempt 0 while an unchanged-address refresh is pending, then delivered, attempt 1, with exact literal HTTPS body and independently verified HMAC after refresh succeeds. The old repro's synthetic public callback was replaced with an owned TLS listener because the repaired path actually sends; its pending/non-terminal assertions were strengthened, not waived.
• Stale-vetted literals and hostnames both refuse connection after becoming internal. egress.rs:528–589 obtains a fresh, healthy lease, rechecks the destination addresses, and retains the lease through reqwest's send. The original literal HTTPS transition probe now returns Forbidden with zero TCP connections; the hostname control also passes. The request was prepared before the deny-set transition.
• The raw-client retargeting gap is removed. egress.rs:398–408 and egress.rs:473–499 expose an immutable destination and sealed POST builder, not a Client or arbitrary-URL method. External compile probes rejected egress::client (E0425), request.post(arbitrary_url) (E0599), and mutation through destination.url() (E0596). A runtime control mutating the caller's original URL to a second owned listener still sent only to the bound TLS callback; the retarget listener received zero TCP.

Contract and lifecycle assessment

The fixed resolver uses the lease's immutable address/deny snapshot and does not reacquire policy locks (egress.rs:418–445). I deliberately queued a writer while retaining a read lease: resolution completed without another DNS lookup or the fair-RwLock reentry deadlock.

Additional independent runtime challenges:

• An actual TLS request held its lease while awaiting response headers. A queued refresh could not invalidate it. 32 parallel short-budget send waiters expired without another callback connection; a longer waiter returned typed Unavailable at the one-second state-wait bound. Aborting the active request released the writer, and the newly internal destination was then refused.
• A 250 ms refresh wait plus TLS response-header stall completed with timeout in 502.384 ms under the original 500 ms send budget. The budget did not restart after lease acquisition; timeout released the lease so a subsequent refresh completed. This is observed cooperative-timer overhead, not a claim of hard real-time precision.
• Cancelling an in-progress refresh preserved prior completed fresh state and allowed the waiting exact TLS body to deliver. Cancelling recovery after a completed failure did not reopen egress.
• Callback DNS failure, one-second DNS timeout, and internal-policy unavailability each kept the delivery retryable with the existing 30-second first backoff, then recovered to delivery at attempt 2 with the original body/HMAC. A callback newly entering the internal deny-set became terminal at attempt 1 without another request. Typed error classification is preserved through hooks.rs:802–845 and webhook_dispatcher.rs:140–199.

Availability tradeoff: active sends can delay refresh until response headers/send completion, bounded by the configured request timeout; a queued writer briefly gates later readers. Those readers fail closed into bounded retry rather than permanent data loss. Freshness is checked at lease acquisition and the checked state remains stable through the send. The absolute sender budget covers send-stage lease/freshness work and transport; initial callback validation/DNS and DB authorization are separate phases, not claimed inside that budget. No additional contract blocker was found in the reviewed or exercised paths.

Exercised verification

All commands used a detached exact-SHA source snapshot under /tmp/tc1132-review-2a3466dd-OJ0KK0ka, offline Cargo, clean explicitly supplied process environment, SQLite memory databases, generated fixture keys/certificates, injected DNS, and owned loopback listeners. Only that disposable snapshot received test-only additions; PR source and other worktrees were not edited.

Commands ran with CARGO_NET_OFFLINE=true, CARGO_TARGET_DIR=$snapshot/target, TMPDIR=$snapshot, and --nocapture --test-threads=1:

Command / scenario	Observed result
Uninstrumented cargo test --offline -p tinycloud-node --lib tc1132_	28 passed, including all original C regressions and the new lease/cancellation/refresh/identity regressions
API-adapted original cargo test --offline -p tinycloud-node --lib independent_	12 passed: ten reviewer probes plus two pre-existing filter matches
--lib egress::independent::lifecycle::	4 passed: queued-writer resolver, 32-way contention/cancel, combined absolute deadline, cancelled refresh states
--lib independent_dns_failure_timeout_and_unavailable_retry_then_recover_but_forbidden_is_terminal	1 passed, with actual TLS recovery/body/HMAC verification
--test did_resolution_routes --test tunnel_service	4 + 14 passed, including default-disabled DID route/socket evidence and tunnel redirect/body/header deadlines plus same-WebSocket recovery
External sealed-API compile-negative probe, --test independent_seal_compile_probe --no-run	Rejected with the three expected compiler errors listed above; fixture retained outside automatic test discovery afterward

The retained independent real TCP Rocket hook-route smoke exercised four simultaneous authorized registrations (200), embedded metadata refusal (400), and four exact TLS/HMAC/content-type/delivery/event/subscription-header deliveries. DNS calls were exactly eight; the owned HTTPS/ALL proxy listener received zero connections. Original TLS controls refused untrusted roots and wrong certificate hostnames. Original alias/NAT64/public-boundary and 32-way stale-refresh controls also passed. Ten actual TLS 503 failures preserved 30/120/600/3600-second backoffs, dead-lettered at attempt 10, and made no eleventh send.

D source is unchanged in this six-path delta. Its full tunnel suite was rerun above; quota redirect/path probes passed in the earlier exact-f30a352 review and were not rerun here. The Part B documentation remains the mandatory future-enable gate, not an implemented did:web fetcher; the current app's independent route suite still reported network methods disabled.

Limits: no independent full PostgreSQL/packet-blackhole suite, SYN-drop test, production build smoke against a public callback, or throughput benchmark. Workspace PostgreSQL/build/fmt/clippy evidence comes from exact-head CI, not from the owner's local report. No children, production/external test targets, operator secret/environment inspection, Docker invocation, host firewall changes, push, merge, or deployment. The zero-match compile-only invocation was not counted as runtime verification. Repro sources and complete local report are retained in the disposable snapshot.

Exact-head CI publication gate: independently verified green

gh run view <id> --repo TinyCloudLabs/tinycloud-node --json headSha,status,conclusion,url returned headSha=2a3466dd31ae7548f892008b4e7cbfabeca6cfae, status=completed, and conclusion=success for all three workflows:

• Rust / workspace build, fmt, clippy and PostgreSQL-backed tests
• Docker Container: job metadata reports ordinary build success; build-dstack and deploy-phala skipped. No deployment was performed by this review.
• Release

APPROVE for this exact candidate only. Both earlier reproduced blockers and the reusable raw-Client gap are closed; no additional code-correctness blocker was found within the stated verification limits. This is a COMMENTED review because the GitHub identity is also the PR author, not a formal approval event. No merge/deployment authorization is implied.

[samgbafa]

APPROVE — independent code-correctness review

Engineering verdict: APPROVE for 2a3466dd31ae7548f892008b4e7cbfabeca6cfae only. This is a COMMENTED review because the shared GitHub identity is the PR author, not a formal approval state. No merge/deploy authorization.

Reviewed the entire 15-path diff against PR base main at f9d700bf7dabe0de401d42168de5890f161f741f, plus registration, dispatcher, stored-delivery authority/status, quota, tunnel, and DID-resolution consumers. The assigned checkout HEAD and live PR head matched this exact candidate before publication. Local main was stale; the reviewed base was the matching PR base / origin/main, not that stale local branch.

No blocking correctness finding remains in the reviewed paths. This verdict comes from my source inspection and independently authored probes below, not earlier reviewers' repros or conclusions.

The two corrected transitions

1. Completed-state refresh / queued delivery: refresh_locked holds exclusive deny-state access while bounded DNS runs, then commits health/timestamp/addresses. It does not expose an interim unhealthy value. My semaphore-gated internal DNS fixture returned the same successful answer: queued hostname and literal deliveries stayed pending, attempts 0, with zero callback TCP while refresh was outstanding; after commit both delivered at attempt 1. Exact UTF-8 body bytes, request path/query, HMAC, content type, and delivery/event/subscription headers were checked at my actual loopback TLS receiver.

   Empty internal DNS and a refresh exceeding its one-second DNS bound each produced retrying, attempt 1, with no new callback connection. Recovery delivered the original bytes at attempt 2. Independently cancelling refresh preserved the prior completed healthy state, but did not lift a previously failed state. Typed temporary errors in the dispatcher preserve retryability; proven forbidden destinations remain terminal.

2. Literal and hostname final enforcement: the sealed sender owns the URL and exposes no raw Client or target setter. Both host classes pass the same fresh, healthy send lease and address check before reqwest is built. My separately prepared literal and hostname sends were vetted before changing the injected internal-service answer to the receiver IP; after the successful refresh, both returned SendError::Policy(Forbidden), with zero TCP connections and no second callback DNS lookup. The test-only loopback class allowance did not override internal denial.

   A separate actual literal TLS request held its lease while the receiver withheld response headers. A queued deny refresh could not commit during that send; the 350ms total send budget released the lease, then refresh committed and new validation returned Forbidden. Existing targeted regressions additionally exercised mixed-address filtering, TLS literal identity, cancellation while waiting/holding a lease, stale failure, the 250ms total budget including refresh wait, and bounded TLS connect stall.

Other consumer contracts

• Registration / retry: authorization remains ahead of callback DNS; raw and canonical URL byte limits and secret byte limits are enforced; HTTPS/shape restrictions remain in production. The exact-source focused tests passed private/embedded refusal, rebinding, corrected public boundaries, unauthorized no-DNS, HMAC/body preservation, sixth-failure hour backoff, and rejection of retry budgets over ten.
• Quota: original SpaceId is encoded as one complete segment; cache/override identity and fallback remain unchanged. My independently constructed SpaceId contained /, .., ?, #, %2F, and UTF-8. For 301/302/303/307/308, the owned billing listener observed a single encoded segment decoding exactly to the original key and the synthetic bearer; the redirect listener received zero connections, fallback was retained, and an override used the original key. Existing tests also covered same-origin redirect refusal.
• Tunnel: existing full tunnel suite passed 3xx status/Location/body passthrough, stalled-header/body request-local errors, and same-WebSocket recovery. My additional cumulative scenario delayed headers 17 seconds, then progressed body bytes every 3 seconds: TCP closed at 30.001656368 seconds, emitted only the request-local body-read timeout error, and the next request returned 200 / ok over the same authenticated WebSocket. This checks the total budget, not merely separate phase stalls.
• DID / Part B: network methods remain disabled by default; all four existing DID route/evidence tests passed. /invoke, /delegate, and /revoke refused network issuers; /info reported the disabled policy/count-only evidence. README, delegation-flow, and changelog make full Part B mandatory before future enablement. The existing network_methods=true option still reaches the unhardened SSI path; do not enable it or use it as rollback. No hardened Part B fetcher is claimed here.

Verification actually run

All Cargo commands used --offline --locked, an explicitly constructed clean environment, synthetic fixture keys/bearers, SQLite memory state, injected DNS, and owned loopback TCP/TLS/WS listeners. Public-address strings were policy inputs only; no external test sockets were opened. Production input files were unchanged. The first three commands used an uninstrumented exact-commit disposable snapshot; the last two used an alternate copied crate root plus my new test module / integration probe, only in scratch.

Command	Observed result
cargo test --offline --locked -p tinycloud-node --lib tc1132_ -- --nocapture --test-threads=1	28 passed, 0 failed
cargo test --offline --locked -p tinycloud-node --lib billing_ -- --nocapture --test-threads=1	4 passed, 0 failed
cargo test --offline --locked -p tinycloud-node --test tunnel_service --test did_resolution_routes -- --nocapture --test-threads=1	14 tunnel + 4 DID passed, 0 failed
cargo test --offline --locked -p tinycloud-node --lib sol_independent_ -- --nocapture --test-threads=1	5 independently authored probes passed, 0 failed
cargo test --offline --locked -p tinycloud-node --test sol_tunnel sol_independent_ -- --nocapture --test-threads=1	1 independently authored cumulative tunnel probe passed, 0 failed

There was one review-harness compile failure, not a product failure: using include!("lib.rs") for the alternate root hit RFC 1560's macro-expanded #[macro_use] shadowing restriction. Copying the exact root text into the alternate file corrected that harness; all probes then compiled and passed. The billing_ run also printed a detached Tokio task's “context ... being shutdown” panic while the suite itself passed. I have not established a production failure from that fixture-teardown output. Cargo reported the existing proc-macro-error2 v2.0.1 future-incompatibility notice.

Own retained evidence/repro text: /tmp/tc1132-sol-2a3466d-3q3u9uyh (sol_review.rs, sol_tunnel.rs, reproduce.py, logs, CI metadata, this review). My own Cargo target and disposable repo/test state will be removed after this verdict; evidence text remains.

Exact-head CI — independently confirmed

gh run view <id> --repo TinyCloudLabs/tinycloud-node --json workflowName,headSha,status,conclusion,url,jobs returned headSha=2a3466dd31ae7548f892008b4e7cbfabeca6cfae, status=completed, conclusion=success for all three:

• Rust / ci 38624053371: all six jobs successful, including workspace build/fmt/clippy, PostgreSQL-backed tests, capabilities, and m1-realdata.
• Docker Container 38624052850: build, ingress image, and Version guard successful. build-dstack, deploy-phala, and rollback audit were skipped.
• Release 38624052746: plan successful; release artifact build/host/publish/announce jobs skipped. Green workflow is not proof that release binaries were built.

Remaining limits / risks

No independent full PostgreSQL suite, packet-drop/SYN-blackhole run, local Docker build, release-binary build, or live IPv6/NAT64 transport was performed. Those must not be inferred from my focused results. IPv6/embedded-address checks here were policy regressions. DNS failure intentionally closes egress until a successful refresh; a sufficiently long outage can exhaust bounded delivery retries. An active send can defer deny refresh through its configured send budget; waiters fail closed rather than connecting. Sequential webhook dispatch and the pre-existing non-atomic subscription cap are outside this correction. The unimplemented Part B future-enable gate remains mandatory.

No production, account/session/secret-file or operator-environment reads, other-worker checkout access, Docker, host firewall changes, PR source edits, push, merge, or deployment. APPROVE for this exact green head only.