Skip to content

🐛 fix(conformance): let both handshake readers at the secrets, dump logs on failure - #201

Merged
scttbnsn merged 2 commits into
dev/v1.6from
fix/150-conformance-live-fixes
Aug 6, 2026
Merged

🐛 fix(conformance): let both handshake readers at the secrets, dump logs on failure#201
scttbnsn merged 2 commits into
dev/v1.6from
fix/150-conformance-live-fixes

Conversation

@scttbnsn

@scttbnsn scttbnsn commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

The first live run of the tri-tool conformance matrix (31053176634, dispatched against ghcr.io/codeswhat/sockguard:1.6.0-rc.1 as the v1.6.0 pre-GA gate) failed all three rows on auth-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.txt was chowned 65532:65532 mode 0400. The same compose secret is read by portwing (UID 65532) and by drydock's runtime user node (UID/GID 1000, post-su-exec) — drydock got EACCES, never authenticated, and the row read as a 90s handshake timeout. Now 65532:1000 mode 0440 (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 against ghcr.io/codeswhat/portwing:latest. The README/overlay keygen instructions gained the missing umask 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's 401 log line. run-matrix.sh --self-test, shellcheck, both compose-combo config --quiet checks, and the 3-case npm test all pass.

After merge: re-dispatch quality-tri-tool-conformance.yml from dev/v1.6 against the rc.1 image to continue the pre-GA gate.

Changelog

  • 🔧 Changed Standard Mode token permissions to 65532:1000 with mode 0440.
  • 🔒 Secured the Edge Mode private key with umask 077 before public-key derivation.
  • 🐛 Added drydock, Portwing, and sockguard log tails on auth-handshake failure.
  • ✨ Updated compose setup, troubleshooting, and key-generation instructions.
  • ✨ Validation passed for the live stack, self-test, ShellCheck, Compose checks, and npm tests.

…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.
@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sockguard-website Ready Ready Preview Aug 6, 2026 12:03am

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@scttbnsn, you've reached your PR review limit, so we couldn't start this review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d5726532-24f9-44ea-a472-bd0e8b555434

📥 Commits

Reviewing files that changed from the base of the PR and between 7787aa3 and a59f76b.

📒 Files selected for processing (1)
  • examples/compose/tri-tool/docker-compose.yml
📝 Walkthrough

Walkthrough

Standard Mode now permits Portwing and drydock to read the shared token with ownership 65532:1000 and mode 0440. Edge Mode applies umask 077 before private-key creation and derives the public key after permission updates. Authentication handshake timeouts now print recent service logs before failure.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/150-conformance-live-fixes

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@scttbnsn

scttbnsn commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 33b9405 and 7787aa3.

⛔ Files ignored due to path filters (1)
  • CHANGELOG.md is excluded by !CHANGELOG.md
📒 Files selected for processing (4)
  • examples/compose/tri-tool/README.md
  • examples/compose/tri-tool/docker-compose.edge-exec.yml
  • examples/compose/tri-tool/docker-compose.yml
  • scripts/tri-tool-conformance/run-matrix.sh

Comment thread examples/compose/tri-tool/docker-compose.yml Outdated
@scttbnsn
scttbnsn merged commit 21b0842 into dev/v1.6 Aug 6, 2026
39 checks passed
@scttbnsn
scttbnsn deleted the fix/150-conformance-live-fixes branch August 6, 2026 00:08
scttbnsn added a commit that referenced this pull request Aug 6, 2026
…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
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