Skip to content

feat: containment for host-executed test packages (npm script deny + … - #8

Merged
franksec42 merged 2 commits into
mainfrom
fix/tenant-id-org-propagation
Jul 2, 2026
Merged

feat: containment for host-executed test packages (npm script deny + …#8
franksec42 merged 2 commits into
mainfrom
fix/tenant-id-org-propagation

Conversation

@franksec42

Copy link
Copy Markdown
Contributor

…bypass detection)

Incident follow-up: a package's lifecycle script executed on a host during testing. Two containment/detection controls, scoped to this repo:

  1. phoenix-firewall init --test-mode bakes an unconditional npm_config_ignore_scripts=true (npm/pnpm — Yarn Classic doesn't reliably honor the env var into every generated shim, applied before the bypass-token check and independent of the firewall verdict or proxy reachability. Even a package that's let through (or a bypass-token host) can't run preinstall/install/postinstall on a test-mode host.

    • internal/shim/generator.go, path_{darwin,linux,windows}.go: threaded DenyScripts through Generator/InstallPATH on all 3 platforms.
    • cmd/init_cmd.go: new --test-mode flag, persisted to agent.toml and agent-bridge.json for audit trail.
  2. Direct-host-install detection: whenever a shim proceeds without firewall evaluation (proxy unreachable, fail_mode=open — i.e. exactly the bypass scenario in the incident), it now always appends to ~/.config/phoenix-firewall/bypass.log, regardless of PHOENIX_FIREWALL_VERBOSE. internal/telemetry/bypass.go drains and counts that log; the endpoint daemon's existing heartbeat (internal/telemetry/heartbeat.go) now reports direct_install_bypass_events on every cycle, since the shim itself has no network access to report this on its own.

Not addressed here (flagged to the user as belonging elsewhere): a disposable test-runner with no inherited credentials, and sandbox proof artefacts (container ID/image digest/transcript) — this repo has no test-runner/sandbox harness at all (checked); those are CI/test-infra concerns, not firewall product code.

Verified: build + vet + test green on darwin/linux/windows cross-compiles (118 tests), plus an end-to-end functional test that actually runs a generated bash shim and confirms bypass.log is written.

EOF
)

franksec42 and others added 2 commits July 2, 2026 14:52
…bypass detection)

Incident follow-up: a package's lifecycle script executed on a host during
testing. Two containment/detection controls, scoped to this repo:

1. `phoenix-firewall init --test-mode` bakes an unconditional
   npm_config_ignore_scripts=true (npm/pnpm — Yarn Classic doesn't reliably
   honor the env var into every generated shim, applied before the
   bypass-token check and independent of the firewall verdict or proxy
   reachability. Even a package that's let through (or a bypass-token host)
   can't run preinstall/install/postinstall on a test-mode host.
   - internal/shim/generator.go, path_{darwin,linux,windows}.go: threaded
     DenyScripts through Generator/InstallPATH on all 3 platforms.
   - cmd/init_cmd.go: new --test-mode flag, persisted to agent.toml and
     agent-bridge.json for audit trail.

2. Direct-host-install detection: whenever a shim proceeds without firewall
   evaluation (proxy unreachable, fail_mode=open — i.e. exactly the bypass
   scenario in the incident), it now always appends to
   ~/.config/phoenix-firewall/bypass.log, regardless of
   PHOENIX_FIREWALL_VERBOSE. internal/telemetry/bypass.go drains and counts
   that log; the endpoint daemon's existing heartbeat (internal/telemetry/heartbeat.go)
   now reports direct_install_bypass_events on every cycle, since the shim
   itself has no network access to report this on its own.

Not addressed here (flagged to the user as belonging elsewhere): a disposable
test-runner with no inherited credentials, and sandbox proof artefacts
(container ID/image digest/transcript) — this repo has no test-runner/sandbox
harness at all (checked); those are CI/test-infra concerns, not firewall
product code.

Verified: build + vet + test green on darwin/linux/windows cross-compiles
(118 tests), plus an end-to-end functional test that actually runs a
generated bash shim and confirms bypass.log is written.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
EOF
)
Local-only test fixture that reproduces the shape of the anthropic-toolkit
incident's postinstall TTP (lifecycle script -> DNS lookup -> outbound HTTPS
call) with none of the actual risk: no filesystem access, no credential/SSH
reads, no data collection, always exits 0. Not wired into any build/CI path
and not published - see README.md in the fixture directory for safety
guarantees and usage (npm pack + install only inside a disposable,
network-isolated environment, beaconing only to infrastructure you control).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 2, 2026 14:37
@franksec42
franksec42 merged commit c8bbed5 into main Jul 2, 2026
1 check passed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new --test-mode flag to unconditionally disable npm/pnpm lifecycle scripts, adds a canary test fixture for postinstall network beacons, and implements bypass logging to track unscanned package installations when the proxy is unreachable in fail-open mode. Telemetry has been added to report these bypass events via the heartbeat payload. The review feedback highlights two critical Windows-specific issues: a potential failure in DrainBypassEvents if a leftover .draining file exists, and a lack of test isolation in bypass_test.go due to os.UserHomeDir() relying on USERPROFILE instead of HOME on Windows.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +32 to +35
tmp := path + ".draining"
if err := os.Rename(path, tmp); err != nil {
return 0
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

On Windows, os.Rename will fail if the destination file already exists. If a previous run crashed or failed to clean up the .draining file, all subsequent calls to DrainBypassEvents will fail on Windows, preventing any future bypass events from being reported. To prevent this, ensure any leftover .draining file is removed before renaming.

Suggested change
tmp := path + ".draining"
if err := os.Rename(path, tmp); err != nil {
return 0
}
tmp := path + ".draining"
_ = os.Remove(tmp)
if err := os.Rename(path, tmp); err != nil {
return 0
}

Comment on lines +10 to +11
home := t.TempDir()
t.Setenv("HOME", home)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

On Windows, os.UserHomeDir() retrieves the home directory using the USERPROFILE environment variable (not HOME). Without setting USERPROFILE in the test, BypassLogPath() will resolve to the developer's real home directory on Windows, which can pollute their environment and cause the test to fail if no bypass log exists there. Setting both HOME and USERPROFILE ensures cross-platform test isolation.

Suggested change
home := t.TempDir()
t.Setenv("HOME", home)
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)

Copilot AI 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.

Pull request overview

This PR adds two containment/detection controls aimed at preventing and detecting host-executed package lifecycle scripts during firewall testing: (1) a --test-mode init flag that bakes “deny npm/pnpm lifecycle scripts” into generated shims, and (2) shim-side logging of “direct host installs” (proxy unreachable + fail-open) surfaced via the agent heartbeat.

Changes:

  • Add phoenix-firewall init --test-mode and thread a denyScripts flag through shim generation on darwin/linux/windows.
  • Record “proxy bypass / fail-open install” events to a local bypass log from shims, and report drained counts via heartbeat telemetry.
  • Add a safe canary npm package fixture that mimics a postinstall network-beacon TTP for detection testing.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
README.md Documents the new init flags, including --test-mode.
internal/telemetry/heartbeat.go Adds direct_install_bypass_events to heartbeat payload.
internal/telemetry/bypass.go Implements bypass log path + drain-and-count helper.
internal/telemetry/bypass_test.go Tests bypass log draining/counting behavior.
internal/shim/path_windows.go Threads denyScripts into Windows shim generation; adds bypass.log append on fail-open.
internal/shim/path_linux.go Threads denyScripts into Linux shim generation.
internal/shim/path_darwin.go Threads denyScripts into macOS shim generation.
internal/shim/generator.go Adds DenyScripts support to bash shim template; appends bypass.log on fail-open.
internal/shim/generator_test.go Tests DenyScripts behavior (npm/pnpm only; ordering before bypass-token check).
internal/shim/bypass_log_test.go End-to-end test: generated shim writes bypass.log when proxy is unreachable and fail-open.
internal/proxy/testdata/canary-packages/phoenix-canary-postinstall-beacon/scripts/postinstall.js Canary fixture script performing DNS lookup + HTTPS POST, always exiting 0.
internal/proxy/testdata/canary-packages/phoenix-canary-postinstall-beacon/README.md Safety and usage documentation for the canary fixture.
internal/proxy/testdata/canary-packages/phoenix-canary-postinstall-beacon/package.json Declares the canary package and its postinstall script.
cmd/init_cmd.go Adds --test-mode, persists it to config artifacts, and passes it into shim install.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +32 to +48
tmp := path + ".draining"
if err := os.Rename(path, tmp); err != nil {
return 0
}
defer os.Remove(tmp)

data, err := os.ReadFile(tmp)
if err != nil {
return 0
}
count := 0
for _, line := range strings.Split(strings.TrimSpace(string(data)), "\n") {
if line != "" {
count++
}
}
return count
Comment on lines +124 to +128
// DenyScripts forces package-manager lifecycle scripts (npm/yarn/pnpm
// preinstall/install/postinstall) off, independent of the firewall
// verdict or proxy reachability. Intended for test-mode installs of
// known-malicious samples, where a script must never execute on the
// host even if the package itself is let through.
Comment on lines +50 to +63
const req = https.request(
{
host: address,
servername: ENDPOINT, // SNI, so TLS cert validation matches the hostname, not the raw IP
port: 443,
path: '/canary',
method: 'POST',
timeout: REQUEST_TIMEOUT_MS,
headers: {
'Content-Type': 'application/json',
'Content-Length': Buffer.byteLength(FIXED_BODY),
'User-Agent': 'phoenix-firewall-canary-postinstall-beacon/0.0.1',
},
},
Comment on lines +25 to +28
fakeHome := t.TempDir()
fakeBinDir := filepath.Join(fakeHome, "bin")
os.MkdirAll(fakeBinDir, 0755)
os.WriteFile(filepath.Join(fakeBinDir, "npm"), []byte("#!/bin/sh\necho real-npm-ran\n"), 0755)
Comment thread cmd/init_cmd.go
Comment on lines 79 to +80
mode = "open"
`, apiURL, apiKey, proxyPort)
`, apiURL, apiKey, proxyPort, testMode)
franksec42 added a commit that referenced this pull request Jul 2, 2026
cmd/version.go declares lowercase unexported version/commit/date vars, but
the Makefile's -X flags targeted cmd.Version/cmd.GitCommit/cmd.BuildDate,
which don't exist -- go build silently no-ops unmatched -X paths instead of
erroring, so `make build`/`make build-all` always shipped "dev
(commit: unknown, built: unknown)" regardless of VERSION/GIT_COMMIT/BUILD_DATE.
.goreleaser.yml already used the correct lowercase names; only the plain
Makefile was stale. Found while rebuilding the binary after merging PR #7/#8.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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