feat(paw-patrol): ReleaseRun — governed merge, rollout watch, and rollback through the computer - #466
Conversation
…lback through the computer Closes the gap where a PR merged through the computer was never watched and rollback was manual. WorkCycle.Complete now declaratively creates a ReleaseRun (guarded entity trigger on release_configured); release_run_lifecycle runs one side effect per trigger on the Computer's sandbox — merge the PR via the GitHub API, one health probe per Check, git revert + push on CheckUnhealthy — and reports only through named callbacks. The watch loop is a kernel state_timeout (30s, reset_on CheckPending) bounded by max_checks. ADR-0005. ARN-394. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
|
@greptile review |
…20->60) 30s is the interval between probes; the total budget is max_checks * 30s. 10 min was too tight for a Railway cold Docker build + release, which can queue several minutes before the new commit starts serving. Default is now 60 checks = 30 min, still overridable per release via ConfigureRelease. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…word field The cut-based extraction produced a bad token (GitHub 'Bad credentials'), caught by the first live run. Use a sed capture of the https credential password, verified to authenticate against the GitHub API on the computer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
… prod Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…dex Sol) Fixes every app-layer finding from the review (P0s + P1/P2), keeping WASM side-effect-only and orchestration declarative. P0: - health_url RCE: strict validate_url() (https + allowlist, rejects quotes/ metacharacters/whitespace); validated before merge AND on every check. - Governance bypass: Cedar create/Request restricted to the patrol-release-service principal; the WorkCycle.Complete->Request entity trigger now runs under that principal (principal = "patrol-release-service"). ConfigureRelease restricted to system/patrol/supervisor/Admin. Any-Agent grants removed. Callbacks + timer-Check limited to the service principals. P1: - Merge reconcile: preflight GETs the PR (base.ref must be "main", else Fail before merging); on an ambiguous PUT it re-reads the PR and treats an already-merged PR as success (idempotent, no lost watcher). - Rollback hardened: isolated owner__repo clone dir, origin verified before mutating, configured git identity, GIT_TERMINAL_PROMPT=0, timeout on every network git call, idempotent (skips if the revert already exists), pushes only the enforced base branch. - Degraded streak: a single degraded probe no longer reverts; rollback needs 3 consecutive degraded-on-new-commit probes (new degraded_streak var). - Liveness: Merging/Requested/Unhealthy get safety state_timeouts -> Fail; removed from allow_indefinite; Watching's Check re-arms on itself so a lost CheckPending still advances. (Durable cross-restart timers = ARN-396.) - max_checks bounded: rejects non-numeric/0, clamps to 240 (2h). - Credentials via `git credential fill` (host-matched, not first-line scrape). P2: - validate_repo rejects '.'/'..'/leading-dot/leading-dash parts. - Computer status must be exactly "Ready" (no fail-open on empty). - matching-sha + non-2xx now counts as degraded (not pending). Out of app scope (ticketed): durable timer delivery (ARN-396), per-repo release serialization lane, and Discord notification wiring (ARN-397 — no declarative outbound-message primitive exists in paw-channels today). Tests 30/30; ReleaseRun + WorkCycle L0-L3; paw-patrol composite PASS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…ithub.com-scoped) git credential fill hangs/returns empty on the sandbox (no configured credential.helper) — caught by the hardened live re-run. Revert to the proven host-scoped sed read of the credential password. 30/30 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…rl globoff, rollback idempotency, CSDL) From the 3-reviewer re-review (Fable verified vs the pinned kernel; the round-1 P0s — RCE + governance bypass — confirmed fixed): - degraded_streak was emitted as a JSON string; the kernel's SetCounterFromParam accepts only numbers and silently dropped it, pinning the streak at 0 so the 3-strike rollback never fired (a degraded release waited the full budget). Emit it as a number. (feature-breaking — unit tests missed it, it fails only across the persistence boundary) - probe curl now uses -g (globoff) so a `[` in health_url is literal, not a curl glob range that would read every probe as status 0. - rollback idempotency now checks only HEAD's message (tip), not all history — a stale historical revert no longer skips a needed rollback while reporting a bogus revert_sha. - CSDL: add DegradedStreak + BaseBranch so the new safety state is auditable. 30/30 tests; spec L0-L3 pass. Remaining (design hardening, tracked): bind the merged commit to the reviewed head SHA; per-attempt mktemp rollback checkout with neutralized git config/hooks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…repo reject (ARN-394/397)
Second hardening round from the re-review:
1. Commit-binding (re-review P0-3): read the PR head sha in read_pr; the merge
PUT pins it ("sha") so GitHub refuses if the head moved (read->PUT TOCTOU).
Optional expected_head_sha (WorkCycle.ConfigureRelease -> release_expected_head_sha,
carried on Request) binds the release to a reviewed commit; merge refuses on
mismatch. Merged head recorded (head_sha on MergeSucceeded) for audit.
2. Rollback isolation (re-review P0-4): revert runs in a fresh mktemp -d checkout
per attempt (trap-cleaned), with GIT_CONFIG_GLOBAL/SYSTEM=/dev/null,
core.hooksPath=/dev/null, commit.gpgsign=false, so a poisoned global config or
repo hook can't execute. Token supplied explicitly in the clone/push URL
(ambient helper is off). Tip-only idempotency retained.
3. Per-repo serialization (ARN-397, reject): before merging, refuse if another
ReleaseRun for the same repo is active (read + own Fail; no cross-entity
dispatch). TOCTOU residual noted; atomic lane entity is the stronger form.
Spec: +expected_head_sha/+head_sha (ReleaseRun), +release_expected_head_sha
(WorkCycle), CSDL updated. 32/32 tests; release_run + work_cycle L0-L3;
composite PASS. ADR-0005 addendum. Not deployed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
Address the surviving findings from the round-3 fresh-context reviews (Codex Sol #2 + Fable). All are in the paw-patrol ReleaseRun app layer; the two kernel-level items are ticketed separately (ARN-396 durable timers, ARN-399 install policy GC). - ARN-397 pagination fail-open: active_release_conflict() now filters server-side on active status AND fails closed when the OData response carries an @odata.nextLink (a truncated page could have hidden a concurrent active run). Per-repo reject stays a read + the run's own Fail — no cross-entity dispatch. - Reconcile-arm binding gap: the ambiguous out-of-band-merge reconcile path now re-checks head_binding_ok, so a merge of an unreviewed head that happened outside the workflow is rejected, not adopted. - head_sha validation: validate_sha(pr.head_sha) before the binding check, so a malformed head from the PR read cannot flow into the merge PUT. - Rollback token no longer in the remote URL: clone/push use a clean https URL plus `-c http.extraHeader=Authorization: Basic $AUTH`, so the token never lands in $DIR/.git/config. - parse_release_runs id keys widened to ["entity_id","Id","id"] to match the kernel row shape; conflict repo compare is case-insensitive (GitHub owner/repo is case-insensitive). - ADR-0005 reset_on doc corrected to ["Check","CheckPending"] to match the shipped spec. Tests: 33 release_run_lifecycle unit tests (added case-insensitive conflict, kernel-row-shape parse, no-token-in-URL rollback), 57 paw_patrol_foundation tests, verify-ioa on release_run + work_cycle, composite verify -s specs — all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
Round-4 (2 Codex Sol + Fable, all vs pinned kernel 724eda61) converged on a regression plus edge classes. Fixes: - ARN-397 case-sensitivity (all 3, P1): the round-3 case-insensitive repo compare was dead code — the server-side `$filter repo eq` is case-sensitive, so a differently-cased active run was dropped before the compare could catch it. Filter on active status only and match repo case-insensitively in code (kernel OData `eq` has no case folding). Active set is tiny (<=1/repo), so status-only is page-safe and still fails closed on pagination. - Base-branch TOCTOU (all 3, P1): the merge PUT pins only the head, not the base. Re-read the PR after merging and refuse MergeSucceeded unless it is now merged into main at the bound head with a valid 40-hex merge sha. This one gate also reconciles an ambiguous PUT, replacing the old reconcile arm. - Squash/reconciled merge un-rollbackable (Codex-2, P1): revert is now parent-aware — `-m 1` for a true merge commit, plain `git revert` for a single-parent (squash/rebase) merge. - parse_release_runs fail-open (Codex-1, P2): a 200 with no `value` array, or a row missing every id key, now errors (fail closed) instead of an empty list. - max_checks upper bound (Codex-1, P3): the degraded-streak path now also rolls back when the probe budget is spent. - validate_sha (Codex-1/Fable, P3): require a full 40-hex sha on success arms; generic error text (it validates head shas too, not only merge_sha). - Doc drift (all 3, P3): ADR + comment now describe the Authorization-header token path and the parent-aware/base-revalidation behavior. Codex-1's second ARN-397 sub-hole (projection lag) is a false positive: Fable verified against the pinned kernel that a `$filter` materializes all candidates before filtering. 4 new unit tests; 37/37 pass; wasm32 release build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…revert safety) Round-5 (2 Codex Sol + Fable, vs pinned kernel 724eda61). Fable (kernel-grounded, empirically tested the revert shell) confirmed all seven round-4 fixes CLOSED and recalibrated the two items Codex called P1 to P2/P3. No P0/P1 confirmed. Fixes: - Confirm binds to the pinned head (Codex-2 P1): the post-merge re-read now requires the merged head to equal the head we pinned in the PUT, so an out-of-band merge of a moved head is refused even when expected_head_sha is unset. Closes the reconcile head-bypass. - Confirm-read resilience (Fable P3): the mandatory post-merge GET is retried a few times so a transient blip / GitHub read-after-write lag no longer strands an actually-merged release as Failed (regression from the round-4 confirm GET). - Rollback revert safety (Fable P2, both Codex): auto-revert only a true merge commit (>=2 parents -> -m 1). A single-parent tip (out-of-band squash/rebase) is refused (exit 3 -> Fail -> escalate) rather than partially reverted while reporting RollbackPushed. Our workflow always merges via merge_method=merge, so the normal path stays fully rollbackable. Verified empirically in a scratch repo. - parse_revert_sha strict 40-hex (Codex-1 P3): removes the legacy 7-40 is_sha so RollbackPushed cannot carry an abbreviated hash. Documented as tracked residuals (not fixable here / need the stronger-ARN-397 atomic per-repo lane entity): base-retarget has no compensation (GitHub merge API has no expected-base — dangerous outcome already prevented, fails visibly), textual (non-canonical) repo identity across a rename, and the guard's coverage bounds at scale / cold start + cross-repo blast radius. 38/38 unit tests pass (1 new); wasm32 release build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
|
@greptile review |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…9312 Align the new ReleaseRun module with the temper-wasm-sdk rev the rest of the tree moved to (724eda61 -> b0c79312, via #467/#469). The module was authored when main pinned 724eda61; every sibling module and wasm-helpers are now on b0c79312, so on the PR merge commit release_run_lifecycle was the only module still on the old rev, producing two temper_wasm_sdk::Context types (E0308) in the os-apps WASM build. Pure rev bump — no code change (module uses temper_wasm_sdk::prelude::*, same as its siblings). 38/38 tests pass and the wasm32 build is clean against b0c79312. Re-verified the review basis: the only commits between the two revs are ARN-255 authz (token verification + operator predicate) and a doc-comment edit in effects.rs; odata/query/filter, state_timeout, effects (SetCounterFromParam + increment ordering), and dispatcher (trigger principal) are functionally unchanged, so the 6-round review holds on b0c79312. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
…tile P1) The health URL is curled from the credentialed computer sandbox. validate_url checked syntax but not the host, so a configured loopback/private/link-local or 169.254 metadata URL could reach internal services (blind SSRF). Add validate_url_host: the host must be a public endpoint — refuse localhost, 127./10./192.168./172.16-31./0., 169.254. (link-local + cloud metadata), IPv6 loopback/ULA/link-local, and bare single-label internal names. Applies on both the merge preflight and every check (validate_url is called on both paths). Also documents in ADR-0005: the ConfigureRelease target-binding residual (caller is scoped to trusted principals; binding the target to the WorkCycle's own artifact is tracked follow-up), and why Greptile's shell-injection and "any Agent can create/Request" findings are false positives against the merged code. 39/39 tests pass (1 new); wasm32 build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
Greptile review disposition (4 × P1)Checked each against the merged code:
|
| fn validate_url_host(url: &str) -> Result<(), String> { | ||
| let rest = &url["https://".len()..]; | ||
| let authority_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); | ||
| let authority = &rest[..authority_end]; | ||
| // Drop any userinfo (user@host); take the host, stripping the port. IPv6 | ||
| // literals are bracketed ([::1]:443) — take what's inside the brackets. | ||
| let host_port = authority.rsplit('@').next().unwrap_or(authority); | ||
| let (host, is_ipv6) = if let Some(after) = host_port.strip_prefix('[') { | ||
| (after.split(']').next().unwrap_or(""), true) | ||
| } else { | ||
| (host_port.split(':').next().unwrap_or(""), false) | ||
| }; | ||
| let host = host.to_ascii_lowercase(); | ||
| if host.is_empty() { | ||
| return Err("release_run_lifecycle: health_url has no host".to_string()); | ||
| } | ||
| let private = if is_ipv6 { | ||
| host == "::1" // loopback | ||
| || host.starts_with("fe80:") // link-local | ||
| || host.starts_with("fc") // unique-local | ||
| || host.starts_with("fd") | ||
| } else { | ||
| host == "localhost" | ||
| || host.ends_with(".localhost") | ||
| || host.starts_with("127.") // loopback | ||
| || host.starts_with("10.") // private | ||
| || host.starts_with("192.168.") | ||
| || host.starts_with("169.254.") // link-local + cloud metadata | ||
| || host.starts_with("0.") | ||
| || is_172_private(&host) | ||
| // A public health endpoint is always an FQDN or IP; a bare | ||
| // single-label name (e.g. "internal") resolves only inside the | ||
| // sandbox's network, so refuse it. | ||
| || !host.contains('.') | ||
| }; | ||
| if private { | ||
| return Err(format!( | ||
| "release_run_lifecycle: health_url host {host:?} is not a public endpoint (loopback/private/link-local/internal)" | ||
| )); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
DNS rebinding bypasses SSRF guard
When an authorized release configuration supplies a dotted HTTPS hostname that resolves to a loopback, private, or link-local address, the textual hostname validation accepts it and Check passes it unchanged to sandbox curl, causing the credentialed computer to probe internal services.
How this was verified: The configured hostname flows through textual validation into sandbox curl without a resolution-level private-address check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: os-apps/paw-patrol/wasm/release_run_lifecycle/src/lib.rs
Line: 907-948
Comment:
**DNS rebinding bypasses SSRF guard**
When an authorized release configuration supplies a dotted HTTPS hostname that resolves to a loopback, private, or link-local address, the textual hostname validation accepts it and `Check` passes it unchanged to sandbox `curl`, causing the credentialed computer to probe internal services.
**How this was verified:** The configured hostname flows through textual validation into sandbox `curl` without a resolution-level private-address check.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
What
Closes the deploy gap found on the 2026-08-23 DSF release: the merge ran as a governed Exec on the computer, but nothing watched the rollout and rollback was manual.
ReleaseRun(paw-patrol):Requested → Merging → Watching → Healthy | Unhealthy → RolledBack | Failed.WorkCycle.ConfigureReleaserecords repo / PR / computer / health URL / probe budget;Completecarries a guarded entity-kind trigger (bool_true release_configured) that creates the ReleaseRun and firesRequestwithparams_from. Declarative — no WASM dispatches it.release_run_lifecycleWASM: one side effect per trigger on the named Computer's sandbox, reporting only via named callbacks:Request→ merge PR via GitHub API (token on the computer) →MergeSucceeded(merge_sha)Check→ onecurlof the health URL →CheckHealthy(2xx + healthy + servedgit_sha == merge_sha) /CheckPending/CheckUnhealthy(degraded on the new commit, or budget spent)CheckUnhealthy→git revert -m 1 <merge_sha>+ push main →RollbackPushed(revert_sha)(same push-triggered deploy path restores the previous build)state_timeoutonWatching(30s,reset_on = ["CheckPending"]). No polling loop in code.os-apps/paw-patrol/adrs/.Pairs with arni-labs/deep-sci-fi#106 (
git_shaon/health), which is the first PR this workflow releases.Verification
release_run_lifecycle: 21/21 unit tests (merge parsing, probe verdicts incl. budget edge, rollback command/sha, shell-safety validators).ReleaseRunspec: L0–L3 pass (17 model states, 100 property cases, 0 unreachable).WorkCyclespec: L0–L3 pass (358 states). Whole-app composite verify: PASS.Linear: ARN-394
🤖 Generated with Claude Code
https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
Greptile Summary
The PR adds a governed ReleaseRun lifecycle that merges an approved pull request, watches its deployed health and commit, and automatically reverts an unhealthy merge. The attempted health-probe SSRF hardening remains bypassable through DNS resolution.
Confidence Score: 3/5
The PR is not yet safe to merge because the health-probe SSRF boundary remains bypassable through attacker-controlled DNS resolution.
A configured dotted hostname can pass the textual host checks, resolve to a private or link-local address, and be curled from the selected computer sandbox without any resolution-level destination enforcement.
Files Needing Attention: os-apps/paw-patrol/wasm/release_run_lifecycle/src/lib.rs
Security Review
The health URL guard blocks literal private hosts but accepts attacker-controlled dotted hostnames without constraining DNS resolution, allowing probes to resolve to private or link-local services. How this was verified: The configured hostname passes textual validation and flows unchanged into sandbox curl, with no resolution-level private-address check.
Important Files Changed
Sequence Diagram
sequenceDiagram participant W as WorkCycle participant R as ReleaseRun participant C as Computer sandbox participant G as GitHub participant H as Health endpoint W->>R: Complete trigger creates and Requests R->>C: release_merge C->>G: Read and merge pinned PR head G-->>R: MergeSucceeded(merge_sha) loop State timeout while Watching R->>C: Check C->>H: curl health_url H-->>R: health status and git_sha end alt Healthy and matching merge_sha R->>R: CheckHealthy else Budget spent or degraded R->>C: CheckUnhealthy C->>G: Revert merge commit and push main G-->>R: RollbackPushed(revert_sha) endPrompt To Fix All With AI
Reviews (4): Last reviewed commit: "fix(paw-patrol): SSRF host guard on the ..." | Re-trigger Greptile