[Jisung Chae (jisung-02)]

Summary

Follow-ups from the #580 review (#409). On macOS the agent now reads the auth.sock peer pid with LOCAL_PEERPID, so the claimed-pid check that Linux already ran applies there too. The check is also on by default now: a peerCred built without the opt-out field keeps it, instead of silently skipping it.

Changes

• peer_cred_darwin.go: read the peer pid with LOCAL_PEERPID beside LOCAL_PEERCRED. A comment notes that, unlike SO_PEERCRED, it reports the last process to use the peer socket, not the connector; the uid stays connect-time.
• peer_cred.go, auth_manager.go: replace pidKnown with skipPIDCheck. Every kernel source reports a pid now, so only the net.Pipe test stand-in (trustedTestPeer) sets it.
• auth_manager.go: the drop log reads "claims a pid other than the peer process", and the verifyPeer doc no longer says "connecting process", since on macOS the pid is not the connector's.
• Tests: the pid tests move from the Linux-only file to the shared linux || darwin file and run on macOS. The drop helper splits in two: before the request is read, Linux may reset the socket (ECONNRESET); after it is read, the close must be clean.

Behavior

• Linux: unchanged apart from the drop log message.
• macOS: a request whose pid does not match the peer, or that has no pid, is dropped. alpamon-pam does not ship for macOS, so no existing client sends these requests.

Testing

• macOS: go test -p 1 -count=1 ./pkg/runner/... passes.
• Linux (golang:1.26 in Docker): ./pkg/runner/... passes except TestGetSystemData, which fails on main as well.
• gofmt -l, GOOS=linux go vet, and GOOS=windows go vet are clean.

Checklist

• Code follows project style guidelines
• Self-reviewed the code
• No hardcoded secrets or credentials
• Tests cover the new macOS pid read and the opt-out default

[Copilot]

🟢 Approved

The reviewed changes are covered by the supplied tests and no unresolved blocking issues were identified.

No open issues 🎉

What changed in this PR

This pull request extends auth.sock peer-PID verification to macOS and enables PID validation by default for kernel-derived credentials.

Changes:

• Reads macOS LOCAL_PEERPID alongside LOCAL_PEERCRED.
• Replaces pidKnown with the default-enabled skipPIDCheck opt-out for test stand-ins.
• Consolidates Linux/macOS PID verification tests and updates connection-drop assertions.

File	Description
pkg/​runner/​peer_cred.go	Defines the default-on PID verification state.
pkg/​runner/​peer_cred_linux.go	Reports Linux kernel peer credentials without the obsolete flag.
pkg/​runner/​peer_cred_darwin.go	Adds macOS peer-PID retrieval through LOCAL_PEERPID.
pkg/​runner/​auth_manager.go	Enforces PID matching and updates peer terminology.
pkg/​runner/​auth_manager_tracker_test.go	Marks net.Pipe test peers as trusted stand-ins.
pkg/​runner/​auth_manager_peer_unix_test.go	Adds shared Linux/macOS PID verification and close-behavior coverage.
pkg/​runner/​auth_manager_peer_linux_test.go	Removes tests relocated to shared Unix coverage.

_{Review effort: Lite}

[alpacax-wall-d]

WALL-D review: Approval withheld · head 7256298

Code review result: Human approval required · head 7256298

Review summary

A maintainer will review this PR.

Product review result: Nothing flagged · head 7256298

Re-review requests and limits

_{WALL-D reviews a PR opened ready after this rule took effect once on its own. After that it reviews only on request: comment @alpacax-wall-d review or mark a draft ready for review, up to 5 times per PR including the first review. A re-review reads only the lines changed since the last review, and from the second re-review on only P1 findings block approval. Answering the product review's questions reviews only the product side again.}

[JunhoAhn (junho226)]

Summary

This PR follows up the #580 review. On macOS the agent now reads the auth.sock peer pid with LOCAL_PEERPID, so the claimed-pid check that Linux already ran applies there too. pidKnown becomes skipPIDCheck, which makes the check default-on: a peerCred built without the field keeps the check, and only the net.Pipe stand-in (trustedTestPeer) opts out.

Critical Issues (0)

Warnings (0)

Suggestions (0)

Highlights

• Flipping the flag to an opt-out closes the failure mode where a new kernel source forgets to set pidKnown and silently skips the check. The only production constructor (peerCreds: peerCredentials in auth_manager.go) and both kernel sources leave skipPIDCheck false; it is set only in auth_manager_tracker_test.go.
• The peer_cred_darwin.go comment records the real semantic difference: xnu answers LOCAL_PEERPID with the peer socket's last user, while LOCAL_PEERCRED stays connect-time. A later reader would otherwise assume SO_PEERCRED behavior.
• Splitting the drop helper is a stricter test, not a looser one. Before the request is read, Linux may reset the socket (ECONNRESET). After it is read, the close must be clean, so assertDroppedAfterReadingRequest keeps require.NoError.
• The pid tests moved to the linux || darwin file, so the macOS pid read now has coverage (TestPeerCredentials_ReportsThisProcess, plus the spoofed-pid and missing-pid drops). The Linux-only file is deleted, so nothing is duplicated.

Notes

• On macOS, verifyPeer samples the pid before handleSudoRequest reads the request. A client that connects in one process and writes from another could therefore be compared against either pid, depending on timing. No macOS auth.sock client exists today, so this is not a defect in this PR. Revisit the order of the sample and the read if a macOS PAM producer is ever added.

---

Verdict: APPROVED

Checklist:

• Code follows stack conventions
• Tests included and passing (CI green on all Linux distros, macos-arm64, windows-amd64, race detector, golangci-lint)
• No security vulnerabilities
• Documentation updated if needed (no CLAUDE.md statement is affected)