Skip to content

feat(sandbox): Landlock PTY + direct TCP + DNS (+ #708 basic auth credential resolve) - #2

Open
kosaku-sim wants to merge 11 commits into
base/upstream-syncedfrom
fix/basic-auth-credential-resolve
Open

kosaku-sim wants to merge 11 commits into
base/upstream-syncedfrom
fix/basic-auth-credential-resolve

Conversation

@kosaku-sim

Copy link
Copy Markdown
Member

Summary

Custom patches on top of NVIDIA upstream for simount's AutoDev sandbox deployment. Base branch base/upstream-synced is a snapshot of nvidia/OpenShell:main (commit 3dd6d51c) to keep the diff clean (5 commits).

Commits (latest first)

  1. feat(sandbox): add PTY devices to proxy-mode baseline read-write paths (6be4e01b)

    • Adds /dev/ptmx and /dev/pts to PROXY_BASELINE_READ_WRITE so Landlock does not block VS Code Remote-SSH's node-pty allocation for integrated terminals.
    • Extends unit tests covering proxy-mode baseline enrichment.
  2. fix: broad TCP 443 ACCEPT instead of per-IP rules for direct hosts (4cb2f388)

    • Replaces per-IP iptables ACCEPT with broad TCP 443 ACCEPT when OPENSHELL_DIRECT_TCP_HOSTS is set. Google API DNS round-robin invalidates per-IP rules.
  3. feat: allow direct TCP 443 for OPENSHELL_DIRECT_TCP_HOSTS (b91ae834)

    • Bypass proxy for HTTPS to pre-declared hosts (Slack Socket Mode endpoints etc.).
  4. fix: add IP forwarding and NAT for DNS through sandbox veth (59ecdd67)

    • Host-side veth forwarding + MASQUERADE so DNS return traffic reaches sandbox netns.
  5. fix: allow UDP DNS to cluster nameserver in sandbox netns (e63ada7c)

    • iptables ACCEPT for UDP 53 to cluster DNS.

Why a fork branch (not upstream PR)

  • Patches 2–5 are specific to Kubernetes cluster DNS + Slack Socket Mode workflows and are tracked in internal docs.
  • Patch 1 is generally useful (node-pty support under Landlock) but needs NVIDIA input on preferred approach (enumerate devices vs broaden baseline) before an upstream PR.

Deployment status

Build `0.0.18-dev.57+g4cb2f388` is currently running in production sandbox. This branch is already rebased on top of upstream PR NVIDIA#708 (L7 credential injection for Basic auth) which resolves git-over-HTTPS 401 for private repos.

Test plan

  • `cargo build --release -p openshell-sandbox` (aarch64 native, Rust 1.88, 4m23s)
  • `git clone https://github.com/simount/.git` from sandbox succeeds (was 401 before deploy)
  • Sandbox pod restart via `autodev-reconcile.sh restart-sandbox` (cred backup/restore)
  • VS Code Remote-SSH integrated terminal allocates PTY without EACCES
  • Unit tests in `baseline_tests` module (local cargo test)

kosaku-sim and others added 11 commits April 11, 2026 06:58
The sandbox iptables rules unconditionally REJECT all UDP traffic,
which blocks DNS resolution for libraries that bypass HTTP_PROXY
(e.g. Node.js ws used by @slack/socket-mode).

Add an ACCEPT rule for UDP port 53 to the nameserver from
/etc/resolv.conf (or OPENSHELL_DNS_SERVER env override) before
the blanket UDP REJECT, so sandboxed processes can resolve
external hostnames without opening a broad UDP hole.

Fixes: NVIDIA/NemoClaw#409
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The DNS ACCEPT iptables rule alone is insufficient because the
sandbox netns routes everything via 10.200.0.1 (host veth).
DNS UDP packets reach the host side but the pod network cannot
route responses back to 10.200.0.2 (sandbox IP).

Enable IP forwarding on the host veth and add MASQUERADE so DNS
packets appear to come from the pod IP, allowing CoreDNS to
respond correctly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Libraries like Node.js ws (used by @slack/socket-mode) resolve DNS
then connect directly to the resolved IP on TCP 443, ignoring
HTTP_PROXY. The sandbox iptables REJECT all bypass TCP, breaking
these connections even after DNS resolution succeeds.

Add OPENSHELL_DIRECT_TCP_HOSTS env var (comma-separated hostnames).
At sandbox netns setup, resolve these hosts and install:
- iptables ACCEPT for TCP 443 to resolved IPs (sandbox side)
- MASQUERADE + FORWARD rules (host side) for return routing

This pairs with the DNS ACCEPT rule from the previous commit to
provide full direct connectivity for proxy-unaware libraries.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DNS round-robin causes Google API IPs to change frequently, breaking
per-IP iptables ACCEPT rules and causing 401/timeout errors. Replace
per-IP filtering with broad TCP 443 ACCEPT when OPENSHELL_DIRECT_TCP_HOSTS
is set — apps still route through HTTPS_PROXY for non-NO_PROXY hosts,
so per-IP iptables filtering adds brittleness without security benefit.

Also adds OPENSHELL_DIRECT_TCP_HOSTS entries to NO_PROXY env var so
HTTP clients skip the proxy for those hosts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
VS Code Remote-SSH launches its server under the sandbox policy, and the
server later allocates PTYs for the integrated terminal via node-pty.
Landlock blocks device-file opens unless explicitly whitelisted, so PTY
allocation fails with EACCES unless both the PTY multiplexer (/dev/ptmx)
and the slave PTY directory (/dev/pts) are writable.

Also extend unit tests: baseline_read_write_includes_core_runtime_and_pty_paths,
enrich_proto_baseline_paths_adds_pty_paths_for_proxy_mode, and
runtime_device_paths_are_not_prepared_for_chown.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a new env var parallel to OPENSHELL_DIRECT_TCP_HOSTS that accepts
host:port pairs (comma-separated). Each pair installs iptables rules:

- sandbox netns OUTPUT: ACCEPT to dest IP / dport
- host POSTROUTING: MASQUERADE for the same dest/port
- host FORWARD: ACCEPT for the same dest/port
- child_env NO_PROXY: host portion added so HTTP clients also bypass

Unlike DIRECT_TCP_HOSTS (broad TCP 443 ACCEPT), this is per-endpoint —
required for:
- non-HTTPS protocols the egress proxy cannot CONNECT-tunnel (postgres
  5432, redis 6379 wire protocols)
- ports the proxy explicitly blocks with ECONNRESET (5432, 6379, 15432,
  16379 observed)
- raw-TCP clients that ignore HTTP_PROXY env

Host component may be an IPv4 literal or hostname; hostnames are resolved
at pod startup via the pod netns resolver. IPv6 addresses are dropped
(sandbox iptables rules are IPv4-only).

Use case: sandbox reaching services on the pod host (postgres, redis,
mailhog) via AUTODEV_HOST_IP without a port-forward daemon inside the
sandbox.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The direct-TCP tests call std::env::set_var / remove_var directly. These are
unsafe in edition 2024, so `cargo test -p openshell-sandbox --lib` failed to
compile (19 errors in netns.rs and child_env.rs) and no sandbox test could run.
Wrap each call in an unsafe block; behavior is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Port the alias resolution from NVIDIA#1286 (header and request-line
scope only). A client that validates the credential's shape before sending
cannot carry the canonical `openshell:resolve:env:KEY` placeholder; for example
gh's `--attach` only accepts github_pat_ / ghp_ / gho_ / ghu_ tokens. Such a
client can send `<prefix>OPENSHELL-RESOLVE-ENV-KEY` (e.g.
`github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN`), and the proxy now resolves it
to the same secret as the canonical placeholder.

- resolve_placeholder falls back to the alias when there is no exact match.
  The alias must be the whole token: a non-empty prefix of RFC 3986 unreserved
  characters, the marker, and an env key running to the end.
- The fail-closed scans after rewriting also reject a remaining alias marker
  in the header block and in the percent-decoded request line, so an
  unresolved alias is never forwarded upstream.

The child env keeps the canonical placeholder; callers opt in to an alias per
invocation (GH_TOKEN=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN gh ...).
Aliases inside Basic credentials are not resolved; git keeps using the
canonical placeholder.

Refs simount/NemoClaw-on-AWS#254

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d request target

The forward-proxy path (rewrite_forward_request) still scanned only for the
canonical placeholder after rewriting, so an unknown alias in a header, or any
alias in the request line, was forwarded upstream. rewrite_target_for_eval also
let an alias in the target through to OPA.

- rewrite_forward_request: reject a remaining placeholder or alias marker in
  the output, and in the raw and percent-decoded request line
- rewrite_target_for_eval: reject an alias marker in the target (raw and
  percent-decoded). Aliases are resolved in headers only, so one in the target
  would never be resolved
- share the marker checks (contains_credential_marker,
  request_line_has_credential_marker) and drop the now-unused
  PLACEHOLDER_PREFIX_PUBLIC

Addresses review: #4 (comment)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…vars

Wrapping set_var / remove_var in unsafe blocks did not meet their safety
contract: tests run in parallel, and the tests in child_env.rs and
sandbox/linux/netns.rs read and write the same variables. Every such test now
holds a shared lock (child_env::lock_direct_tcp_env) for its whole body,
including the ones that only read through build_no_proxy / proxy_env_vars.

Addresses review:
#4 (comment)
#4 (comment)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
feat(sandbox): 接頭辞付きの credential 別名を placeholder と同じ秘密に解決する
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.

2 participants