Skip to content

Network/env follow-ups (#177) and runner-owned adapter deadline (#175) - #179

Merged
howlcipher merged 7 commits into
mainfrom
fix-177-175
Oct 13, 2026
Merged

howlcipher merged 7 commits into
mainfrom
fix-177-175

Conversation

@howlcipher

Copy link
Copy Markdown
Owner

Closes #177. Closes #175.

Follow-ups from the #176 reviews (Lain, Motoko) and the #173 review (Motoko). Every fix has a negative test, and each was mutation-checked: I reverted or weakened the fix locally and confirmed the test failed.

#177

  • process:env: one deny-list, capability.CodeLoadingEnvNames + CodeLoadingEnvPrefixes, rendered into generated Go and JS. It adds GIT_*, BASH_FUNC_*, LUA_*, GCONV_PATH, PYTHONHOME, PERL5DB, PERLLIB, JAVA_TOOL_OPTIONS/_JAVA_OPTIONS/JDK_JAVA_OPTIONS, SHELLOPTS/BASHOPTS/PS4, HOME/XDG_CONFIG_HOME, PATH, GOFLAGS/GODEBUG, NODE_EXTRA_CA_CERTS, TLS trust stores and more. A parity test runs one independent list (nettest.EnvRefused/EnvAllowed) on core, generated Go and Node, and both rendered tables are compared with the capability tables.
  • Headers: when a redirect changes origin, every header outside capability.RedirectSafeHeaders is stripped. That covers Proxy-Authorization and custom auth headers. Same-origin redirects keep all headers.
  • Userinfo: redirect targets that carry userinfo are refused on all three targets.
  • Node: vets every resolved address and pins the whole set (honoring all and family), not just the first one.
  • [::ffff:a.b.c.d]: core, generated Go and Node now treat it as the same host as a.b.c.d (nettest.MappedGrantCases).
  • Address table: ::ffff:0:0/96 is decoded, and 3fff::/20 and 5f00::/16 are added. A test checks that the generated Go and JS tables equal capability.SpecialPrefixes, and the blocked and public address lists now run against generated Go too.
  • localhost: the opt-out is pinned to loopback only, on all three targets.

#175: adds vm.ExecutionPolicy.AdapterDeadline, which the runner owns. It defaults to 1s and is clamped at vm.MaxAdapterDeadline (30s). A shorter Deadline still wins. vm.AdapterDeadlineFlag / ParseAdapterDeadline give runners an -adapter-deadline flag that rejects 0, negative and over-cap values. Artifact runs are unaffected.

Breaking changes (see change_log.md):

  • process:env=HOME, PATH, GIT_* and the other listed names are now refused. Two existing tests that asserted HOME was accepted now assert LANG instead.
  • Cross-origin redirects drop every non-safe header.
  • Redirects to targets with userinfo are refused.
  • A localhost that resolves to a non-loopback address is refused.
  • Node now matches mapped IPv4 literals.

Pins, freeze, lock, arity and parity are unchanged. Local runs of gofmt, vet and go test ./... are green. Journal: docs/journals/2026-10-13_network_env_followups_adapter_deadline.md

@howlcipher howlcipher left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Assurance review: #179 @ 6da850e (closes #177, #175)

Verdict: ready to merge, no blockers. Every #177 item holds on core, gogen and Node, and the #175 cap is correct. go build ./... and the full go test ./... pass locally, and CI build is green.

Verified

  • Range table. There's one source, capability.SpecialPrefixes (destinations.go:16-21). gogen and JS render it, and parity tests compare each rendered table to the source. I ran an independent 38-blocked / 8-public probe on all 3 targets. Denied: CGNAT, 0/8, broadcast, ::7f00:1, ::a00:1, 64:ff9b:1::/48, 6to4 wrapping private, link-local, multicast and broadcast addresses, Teredo (including the inverted client), 198.18/15, 192.0.0/24, fec0::/10, plus loopback, RFC1918, ULA, link-local, NAT64 /96, ::, multicast and mapped forms. Allowed: 8.8.8.8, 2606:4700::1111, 1.1.1.1, ::ffff:8.8.8.8, ::808:808, and 6to4 wrapping public IPs. Teredo is refused outright, which also blocks public IPs inside it. That's an over-block by design and it's documented.
  • Cross-origin headers. The allowlist (network.go:468-486, gogen.go:963-972, javascript.go:650) keeps 7 safe headers and drops everything else, including non-canonical names like x-api-key, X-AUTH-TOKEN and proxy-authorization. Same-origin redirects keep custom and auth headers. Redirects to URLs with userinfo are refused on all 3 targets.
  • Env deny-list. It's single-sourced (process.go:94-138), case-insensitive, and has a parity test on each target. The independent nettest.EnvRefused list catches anything dropped from the source table. All requested names and prefixes are refused. PATH can't be granted, and it's moot for choosing the program because argv must be an absolute path.
  • localhost. checkDial (network.go:285-310) denies localhost resolving to 169.254.169.254, 10/8, mapped 169.254 or a public address, and allows 127/8, ::1 and ::ffff:127/8. A fake-resolver test covers this on core and gogen, and JS checks and pins every resolved address.
  • #175. ParseAdapterDeadline refuses 0, negative and >30s. effectiveExecutionPolicy clamps to 30s, and a shorter runner deadline wins.

Mutations (my own, not in the PR's set): 17/19 caught. Survived: core and gogen header allowlist matched case-sensitively. This fails closed, because canonical safe names still match and a non-canonical safe header would only be dropped, so it's a test gap rather than a hole. Suggested fix: add one test with a non-canonical safe key.

Follow-ups (not blocking this PR)

  1. Exec isn't contained when the deadline hits (vm.go:2521-2531, existing code). CommandContext plus WaitDelay with no Setpgid or group kill. With a 500ms deadline, sh -c "setsid sleep & sleep" returned in about 600ms (good, no blocking), but both the setsid grandchild and the same-group background child kept running. This must be fixed before any live run grants process:exec: set Setpgid, kill the group in cmd.Cancel, and contain setsid escapees with a cgroup or tracking, unless the harness does that itself.
  2. AdapterDeadlineFlag isn't connected to any runner binary yet. It's only used in the library and tests. Please connect it in the #178 runner.
  3. Gaps in the env deny-list. Code loading: OPENSSL_CONF, OPENSSL_MODULES/ENGINES, RUBYGEMS_GEMDEPS, GEM_PATH, PHPRC, R_PROFILE_USER, TCLLIBPATH, GUILE_LOAD_PATH, ERL_LIBS. Low risk: NETRC, CURL_HOME, WGETRC, NPM_CONFIG_, PIP_, GOPROXY/GOSUMDB/GOINSECURE, KUBECONFIG, GLIBC_TUNABLES, SSLKEYLOGFILE, MAKEFLAGS. Longer term, I'd switch process:env= to a short allowlist, since a denylist will keep leaking.

Assurance (bot-assurance-0001)

@howlcipher howlcipher left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

COMMENT review at 6da850e (base e36bd2d). Verdict: merge-ready. No blockers; three non-blocking items below.

Gates (fresh checkout): gofmt -l empty (exit 0), go vet ./... exit 0, go test -count=1 ./... exit 0 (75 packages ok, including lock/arity 386/parity/freeze/pin tests and the Node-backed JS tests). No default flip, #90 stays Partial, tip-lock untouched.

Env: One table (CodeLoadingEnvNames + prefixes LD_/DYLD_/GIT_/BASH_FUNC_/LUA_), upper-cased before matching, so path/Path/git_ssh_command are refused. validEnvName rejects =, NUL and other non-identifier bytes. Every name on my list is covered except the proxy variables and TMPDIR (see nit 2).

Redirects: Cross-origin keeps only the 7 RedirectSafeHeaders (case-insensitive), so Authorization, Cookie, Proxy-Authorization and X-Api-Key are dropped; same-origin keeps everything. Go refuses userinfo whenever URL.User != nil, including @, :@ and %40 forms. Node checks every resolved address and rejects the set if any member fails; localhost must be loopback on all three targets. ::ffff:0:0/96 is decoded, and 3fff::/20 and 5f00::/16 are in the shared table.

Deadline: effectiveExecutionPolicy defaults to 1s and clamps the policy to 30s. ParseAdapterDeadline refuses 0, negative and >30s rather than clamping. The policy only comes from the runner ExecutionPolicy; no candidate or transport field reaches it. Artifact runs are unaffected (test covers it).

Mutations spot-checked (3/3 caught): dropping PATH from the table fails TestEnvGrantRefusesCodeLoading, TestGoFetchSecurity and TestJSCodeLoadingEnv; disabling the userinfo check fails TestFetchSecurity; removing the 30s clamp fails TestAdapterDeadlineRunnerOwned.

Breaking changes: all 7 are in change_log.md.

Non-blocking:

  1. -adapter-deadline isn't registered by any CLI: AdapterDeadlineFlag is only used in a test. Either wire it into the runner (main -run-bc / the hflive harness) or leave #175 open until that happens.
  2. HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY (and lowercase), NODE_USE_ENV_PROXY, GEM_HOME/GEM_PATH and TMPDIR aren't refused. A granted proxy var would route an allow-listed child's traffic, so consider adding them in a follow-up.
  3. JS userinfo parity: WHATWG gives http://@h/ and http://:@h/ an empty username and password, so the JS check accepts them where Go refuses. They carry no credentials so it's harmless, but checking for @ in the authority would match Go.

@howlcipher
howlcipher marked this pull request as ready for review October 13, 2026 14:45
@howlcipher
howlcipher merged commit 0395010 into main Oct 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant