🔧 chore(release): close post-merge audit gaps - #155
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
biggest-littlest
left a comment
There was a problem hiding this comment.
Verified the socket probe, pinned tri-tool defaults, UID contract, and listener comparison follow-ups.
ALARGECOMPANY
left a comment
There was a problem hiding this comment.
Reviewed the post-merge findings against current code and confirmed the applied fixes and documented dismissals.
Publishes #150's Sockguard + Portwing + drydock conformance matrix. New weekly/workflow_dispatch quality-tri-tool-conformance.yml boots the audited examples/compose/tri-tool bundle from published images only (never source-built) across three rows -- current-standard, current-edge, and legacy-floor (the audited 0.8.1/1.5.2 pins from PR #155) -- and runs scripts/tri-tool-conformance/run-matrix.sh's ten ordered assertions per row: pristine fresh-volume boot, auth handshake plus one negative probe per mode, inventory/inspect, events, logs, lifecycle, configured exec (Edge only), the remote-update trigger, expected denials, and a route-drift tripwire that diffs sockguard's own access log against a checked-in known-routes.json manifest. - 🧪 scripts/tri-tool-conformance/{run-matrix.sh,lib.sh}: the driver + shared helpers; --self-test exercises the route normalizer and tripwire diff logic against a fixture with no Docker required - 🧪 scripts/tri-tool-conformance/normalize-routes.jq + known-routes.json: route-shape normalizer and the manifest seeded from app/configs/portwing.yaml / portwing-with-exec.yaml's allow rules - 🧪 scripts/tri-tool-conformance-run-matrix.test.mjs: wires --self-test into `npm test` - 🔧 examples/compose/tri-tool/docker-compose.conformance-overlay.yml: test-only probe container overlay; the audited bundle itself is untouched - 🔧 examples/compose/tri-tool/{docker-compose.yml,docker-compose.edge-exec.yml,README.md}: rename SOCKGUARD_VERSION (tag-only) to SOCKGUARD_IMAGE (full ref) so the harness and the new pre-GA release gate can target a release-candidate image on any of sockguard's three registries - 📝 CHANGELOG.md: Tests + Docs entries under [Unreleased]
* 🧪 test(conformance): tri-tool conformance matrix workflow + harness Publishes #150's Sockguard + Portwing + drydock conformance matrix. New weekly/workflow_dispatch quality-tri-tool-conformance.yml boots the audited examples/compose/tri-tool bundle from published images only (never source-built) across three rows -- current-standard, current-edge, and legacy-floor (the audited 0.8.1/1.5.2 pins from PR #155) -- and runs scripts/tri-tool-conformance/run-matrix.sh's ten ordered assertions per row: pristine fresh-volume boot, auth handshake plus one negative probe per mode, inventory/inspect, events, logs, lifecycle, configured exec (Edge only), the remote-update trigger, expected denials, and a route-drift tripwire that diffs sockguard's own access log against a checked-in known-routes.json manifest. - 🧪 scripts/tri-tool-conformance/{run-matrix.sh,lib.sh}: the driver + shared helpers; --self-test exercises the route normalizer and tripwire diff logic against a fixture with no Docker required - 🧪 scripts/tri-tool-conformance/normalize-routes.jq + known-routes.json: route-shape normalizer and the manifest seeded from app/configs/portwing.yaml / portwing-with-exec.yaml's allow rules - 🧪 scripts/tri-tool-conformance-run-matrix.test.mjs: wires --self-test into `npm test` - 🔧 examples/compose/tri-tool/docker-compose.conformance-overlay.yml: test-only probe container overlay; the audited bundle itself is untouched - 🔧 examples/compose/tri-tool/{docker-compose.yml,docker-compose.edge-exec.yml,README.md}: rename SOCKGUARD_VERSION (tag-only) to SOCKGUARD_IMAGE (full ref) so the harness and the new pre-GA release gate can target a release-candidate image on any of sockguard's three registries - 📝 CHANGELOG.md: Tests + Docs entries under [Unreleased] * 📝 docs(releasing): pre-GA tri-tool conformance gate Adds RELEASING.md step 4: before promoting a minor/major release candidate to GA, run the new quality-tri-tool-conformance.yml workflow against the candidate image (sockguard_image dispatch input) and require all three matrix rows green. * 🔒 security(ci): fix tri-tool conformance gate image default + summary short-circuit - sockguard_image dispatch input now defaults to empty (falls through to the compose bundle's own pinned default, same as a scheduled run) instead of ghcr.io/codeswhat/sockguard:latest, which pointed at a different registry/tag than the compose default a scheduled run uses. Input description and RELEASING.md's pre-GA gate step now say to pass the candidate ref explicitly. - summary job now asserts artifact count == matrix row count BEFORE aggregating, not just checking for total emptiness after -- a row that dies before write_artifact runs no longer passes the gate silently. * 🔒 security(ci): pin probe container UID to match sockguard's socket owner curlimages/curl's own image default UID (100) can't reach a socket owned 65532:65532 mode 0600, so the probe's curl calls were failing on their own filesystem permission mismatch instead of exercising the socket mode the assertion actually cares about. * 🔒 security(script): allowlist sockguard_image before it's ever exported --sockguard-image gets interpolated into a compose file and pulled/run. The workflow that drives this script is workflow_dispatch/schedule only (no pull_request trigger, so the input is maintainer-controlled, not attacker-controlled via a PR), but validating it against the allowlisted sockguard registries/repos is defense in depth worth having regardless. * 🐛 fix(conformance): tolerate malformed access-log lines, fail closed on zero routes - wait_for_access_log_route's jq filter (lib.sh) now skips non-object JSON and missing/null normalized_path instead of erroring on them. Under `set -o pipefail`, a jq error on any line -- even one arriving after a real match was already printed -- flipped the whole pipeline's exit status non-zero and reported the wait as failed regardless of what grep actually found, poisoning assertions 3 (inventory-inspect) and 4 (events). - normalize-routes.jq gets the same select(type=="object") guard: a bare-string JSON line would otherwise abort the whole `[inputs | ...]` collection rather than just being skipped, poisoning assertion 10. - assert_route_drift now fails closed (route_drift_status, moved to lib.sh so --self-test can exercise it directly) when zero access-log records were captured, instead of recording a vacuous empty-diff PASS -- that almost always means log capture or the normalizer broke, not that nothing happened. - Extended testdata/access-log-fixture.jsonl with a missing-normalized_path line and a bare-string JSON line, and added self-test coverage for both the jq tolerance and the fail-closed behavior. - Fixed lib.sh's stale compose_cmd doc comment (the function is `compose` and runs directly, it doesn't echo an invocation for callers to expand). * 🐛 fix(conformance): pre-pull busybox pins, checked chown, exact digests, aligned skip names - Pre-pull OLD_BUSYBOX_REF and NEW_BUSYBOX_REF explicitly before any assertion needs them, checked, instead of letting the first containers/create implicitly trigger the pull -- a slow/flaky pull on a shared runner otherwise reads as an unrelated timeout in whichever assertion needed the image first. Corrected the header comment: only NEW_BUSYBOX_REF matches app/integration/helpers_test.go's busyboxPinnedRef, and that repo's own pre-pull is for its own CI job, not this one. - portwing_token.txt (standard mode) is now chown'd to 65532:65532 before the chmod 0400 -- it stayed owned by the script runner while portwing reads it as UID 65532. Every chown/chmod in the secrets-setup block (standard and edge) is now checked explicitly instead of `&&`-chained silently, failing the row with a clear setup-error message instead of surfacing later as a confusing auth-handshake failure. - Image digests are now captured via `docker image inspect` right after `compose up` in assert_pristine_boot, from the images the row actually ran. resolve_metadata no longer does a `docker compose pull` at the end of the row, which could catch an upstream tag having moved mid-run and record a digest that never ran any of this row's assertions. - assert_logs no longer pre-emptively records a "lifecycle" FAIL when the primary sentinel create fails -- assert_lifecycle already records its own FAIL for that case, so the artifact was getting a duplicate entry. - The abort-path skip loop (pristine-boot/auth-handshake failure) now emits exactly the same assertion names the success path does (exec-policy-allowed/-denied instead of exec-policy on exec rows, expected-denials-build/-exec/-export instead of expected-denials), and adds a skipped "auth-handshake" entry for the case where pristine boot itself is what failed and auth-handshake was never even attempted.
Summary\n\nCloses the valid post-merge review gaps from PR #154:\n\n- probes the fresh named-volume socket itself, verifies owner/mode, and sends a real /_ping through it\n- pins the tri-tool defaults to the audited Sockguard 1.5.1, Portwing 0.8.1, and drydock 1.5.2 releases while keeping explicit version overrides\n- documents the owner-only UID 65532 consumer contract without weakening the 0600 socket\n- adds the missing multiple-listener column to the website overview matrix\n- fixes the stale v1.5.0 test comment\n\nThe Alpine builder comment was not applied: the shipped runtime stage is already the pinned Chainguard static image, while the builder is intentionally native-platform Go Alpine for reliable cross-compilation. The comparison-row ordering finding was also stale; the terminal-row test passes and the current ordering is intentional.\n\n## Verification\n\n- full Lefthook pre-push pipeline\n- GoReleaser snapshot\n- golangci-lint\n- Go race tests\n- govulncheck\n- fuzz smoke\n- npm dedupe and Knip\n- Biome\n- 92 TypeScript/script tests\n- docs and website production builds\n- zizmor\n- tri-tool Compose resolves the audited version pins