Skip to content

fix(supervisor): enforce peer-UID check in SupervisorSocket::bind() - #1639

Merged
SequeI merged 2 commits into
mainfrom
fixPeer
Aug 13, 2026
Merged

fix(supervisor): enforce peer-UID check in SupervisorSocket::bind()#1639
SequeI merged 2 commits into
mainfrom
fixPeer

Conversation

@SequeI

@SequeI SequeI commented Aug 13, 2026

Copy link
Copy Markdown
Member

Linked Issue

Closes #

Summary

SupervisorSocket::bind() accepted connections without verifying the peers UID, unlike SupervisorListener::accept() which already checked it via SO_PEERCRED/getpeereid. Extract the check into a shared check_peer_uid() helper and apply it in both accept paths so identity verification doesnt depend solely on filesystem permissions/umask timing.

Test Plan

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates

SupervisorSocket::bind() accepted connections without verifying the
peers UID, unlike SupervisorListener::accept() which already checked
it via SO_PEERCRED/getpeereid. Extract the check into a shared
check_peer_uid() helper and apply it in both accept paths so identity
verification doesnt depend solely on filesystem permissions/umask
timing.

Signed-off-by: Aleksy Siek <aleksy@nolabs.ai>
@github-actions github-actions Bot added bug Something isn't working nono size/small labels Aug 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +20
Lines removed -9
Total changed 29
Classification Small (< 50 lines)

Affected crates

  • crates/nono (core library) — careful review required. This is the security-critical sandbox primitive. A bug here bypasses OS-level isolation for every downstream user.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@nogent-nolabs-ai nogent-nolabs-ai 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.

nogent code review

No blocking issues; 0 findings.

Findings: none flagged in scope.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

@SequeI
SequeI merged commit 149579a into main Aug 13, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working nono size/small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants