🐛 fix(conformance): let both handshake readers at the secrets, dump logs on failure - #201
Conversation
…ogs on failure First live matrix run (31053176634) failed all 3 rows on auth-handshake. Local repro against the published images found two perms bugs, not tool bugs: - 🐛 fix(conformance): standard token 65532:1000 mode 0440 -- the same compose secret is read by portwing (UID 65532, owner bit) AND drydock's node user (UID/GID 1000, group bit); 65532:65532 0400 locked drydock out with EACCES and read as a handshake timeout - 🐛 fix(conformance): chown/chmod the edge Ed25519 pem BEFORE keygen -pub-from -- the shell redirect writes it 0644 and portwing's key loader refuses group/world-readable private keys - 🐛 fix(conformance): dump drydock/portwing/sockguard log tails when auth-handshake fails -- the first live failure was undiagnosable from the CI job log - 📝 docs(examples): fix the identical chown bug in the audited bundle's own instructions (compose header + README), add the missing umask 077 to the edge keygen flow, and document the EACCES failure mode - 📝 docs(changelog): record the fix under [Unreleased] Validated: handshake + 'Handshake successful' log line + wrong-secret 401 probe all confirmed against live portwing 0.9.2 / drydock latest / sockguard 1.6.0-rc.1 with a readable token; keygen 0400 acceptance tested against ghcr portwing:latest.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 selected for processing (1)
📝 WalkthroughWalkthroughStandard Mode now permits Portwing and drydock to read the shared token with ownership Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/compose/tri-tool/docker-compose.yml`:
- Line 32: Update the ownership value in the comment near the runtime user
instruction to use 65532:1000 consistently, matching the requirement referenced
at line 35, and remove the stale 65532:65532 value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 87c024c9-7279-400f-a058-4ded4fdca33e
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (4)
examples/compose/tri-tool/README.mdexamples/compose/tri-tool/docker-compose.edge-exec.ymlexamples/compose/tri-tool/docker-compose.ymlscripts/tri-tool-conformance/run-matrix.sh
…ue can't read as the instruction
…shake expectation (#202) * 🐛 fix(conformance): fix negative-probe secret perms and the edge handshake expectation Second live matrix run (31058731379): main auth-handshake green on all rows after #201, three more harness bugs surfaced, all pinned against live published images: - 🐛 fix(conformance): wrong-secret probe's mktemp file is 0600 runner-owned -- the throwaway drydock EACCESes as node (1000) before sending the request that would 401; the garbage value is now chmod 0644 - 🐛 fix(conformance): edge probe's throwaway key gets chowned 65532:65532 so the throwaway portwing can actually read it (same mktemp lockout) - 🐛 fix(conformance): edge rows assert 'Edge agent connected' -- drydock's WS path never emits standard mode's 'Handshake successful' line; the unknown-key probe now reads 'rejected hello|unknown-key' from the throwaway portwing's logs because drydock rejects bad hellos silently (error frame to client, no server-side log; verified in portwing-ws source and against a live rig) - 📝 docs(examples): README edge section rewritten with the verified log lines ('Edge agent connected: portwing-edge-<id>', portwing-side 'controller rejected hello ... (unknown-key)', drydock's world-readable-keys startup warning) - 📝 docs(changelog): record round 2 under [Unreleased] * 🐛 fix(conformance): address unknown-key probe review comments - 🐛 fix: require 'rejected hello.*unknown-key' on one line so a bad-signature rejection can't falsely pass the unknown-key probe - 🐛 fix: sudo rm the chowned throwaway key on both post-chown cleanup paths (sticky /tmp blocks the runner from unlinking a 65532-owned file) - 📝 docs: edge README now distinguishes bad-signature (registered key doesn't match the private key) from unknown-key, and states the authorized_keys chown/chmod command explicitly in the startup warning
The first live run of the tri-tool conformance matrix (31053176634, dispatched against
ghcr.io/codeswhat/sockguard:1.6.0-rc.1as the v1.6.0 pre-GA gate) failed all three rows onauth-handshake. Local reproduction against the published images traced both failures to file permissions in the harness (and the bundle's own docs), not to sockguard/portwing/drydock:Standard rows (current-standard, legacy-floor):
portwing_token.txtwas chowned65532:65532mode0400. The same compose secret is read by portwing (UID 65532) and by drydock's runtime usernode(UID/GID 1000, post-su-exec) — drydock got EACCES, never authenticated, and the row read as a 90s handshake timeout. Now65532:1000mode0440(owner bit for portwing, group bit for drydock). The audited bundle's compose header and README documented the identical broken chown — fixed there too, with the EACCES failure mode added to the troubleshooting line.Edge row: the Ed25519 pem was written by shell redirect (default umask → 0644) and then fed to
keygen -pub-from, which refuses group/world-readable private keys (FATAL: ... unsafe permissions). The chown/chmod now happens before the derivation; 0400 acceptance was tested againstghcr.io/codeswhat/portwing:latest. The README/overlay keygen instructions gained the missingumask 077.Observability: the handshake assertion now dumps drydock/portwing/sockguard log tails on failure — the first live failure was undiagnosable from the CI job log alone.
Validated locally against the live stack (portwing 0.9.2, drydock latest, sockguard 1.6.0-rc.1):
Handshake successful. Received 13 containers.with a readable token, and the wrong-secret probe's401log line.run-matrix.sh --self-test, shellcheck, both compose-comboconfig --quietchecks, and the 3-case npm test all pass.After merge: re-dispatch
quality-tri-tool-conformance.ymlfromdev/v1.6against the rc.1 image to continue the pre-GA gate.Changelog
65532:1000with mode0440.umask 077before public-key derivation.auth-handshakefailure.