Skip to content

fix(cli): reject upstream proxy with block net#1392

Open
dpersek wants to merge 4 commits into
nolabs-ai:mainfrom
dpersek:fix/network-block-upstream-proxy
Open

fix(cli): reject upstream proxy with block net#1392
dpersek wants to merge 4 commits into
nolabs-ai:mainfrom
dpersek:fix/network-block-upstream-proxy

Conversation

@dpersek

@dpersek dpersek commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Related to #1233

Summary

This PR makes hard network blocking reject upstream proxy configuration consistently instead of allowing proxy-mode semantics to win silently.

  • reject --block-net / profile network.block when --upstream-proxy or profile upstream_proxy is present
  • share the upstream-proxy presence check with upstream-bypass validation
  • run the same proxy-conflict validation for nono shell dry-run and launch paths before proxy launch options are built
  • update a stale runtime comment that still described profile network.block as yielding to proxy config

Agent Disclosure

This PR was prepared by an AI coding agent working with Dustin.

Before implementation, Dustin posted the repository-required issue-disclosure/intent/approach comment on #1233. I reviewed AGENTS.md, CLAUDE.md, CONTRIBUTING.md, SECURITY.md, .github/pull_request_template.md, crates/nono-cli/data/profile-authoring-guide.md, docs/cli/features/networking.mdx, and the affected Rust modules. This PR stays within the disclosed scope and does not reuse or adapt external code.

Repository-rule checks:

  • no new production unwrap() or expect() calls
  • new conflict errors use NonoError::ConfigParse
  • tests cover the new security-sensitive configuration conflict
  • the profile authoring guide already documents that network.block: true cannot be combined with proxy settings, so no docs change was needed

Test Plan

  • cargo test -p nono-cli shell_dry_run_rejects_block_net_with_upstream_proxy
  • cargo test -p nono-cli upstream_proxy
  • cargo test -p nono-cli block_net
  • make fmt-check
  • make clippy
  • make test-cli

Notes:

  • make test-cli was run outside the local Codex sandbox after a sandboxed run failed on local supervisor/attach socket binds with Operation not permitted.
  • Full make ci was not run locally; make test-cli, make clippy, and make fmt-check passed.

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
  • Release note has been added to CHANGELOG.md if needed

Agent Compliance Check

  • I am not prohibited from contributing under this policy
  • An issue already exists
  • I disclosed that I am an agent in the issue discussion
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths
  • This PR matches the approved or disclosed issue scope

Signed-off-by: Dustin Persek <dustin.persek@protonmail.com>
@github-actions github-actions Bot added bug Something isn't working nono-cli size/medium labels Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +176
Lines removed -11
Total changed 187
Classification Medium (50–300 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@dpersek dpersek marked this pull request as ready for review July 9, 2026 02:35

@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 suggestions.

Findings: none flagged in scope.

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

dpersek added 2 commits July 9, 2026 12:57
…pstream-proxy

Signed-off-by: Dustin Persek <dustin.persek@protonmail.com>
Signed-off-by: Dustin Persek <dustin.persek@protonmail.com>
Keep the synthetic HOME and allowed working directory as siblings so Linux Landlock validation reaches the upstream-proxy conflict assertion without an allow/deny overlap.

Signed-off-by: Dustin Persek <dustin.persek@protonmail.com>
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-cli size/medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant