Add execution boundary assurance

[bomly-guy]

What changed

• verify managed plugins receive only selected protocol, plugin, and proxy settings instead of unrelated host secrets
• prove disabled native plugins do not join the runtime registry
• exercise scan, explain, and diff MCP calls with hostile text while enrichment, audit, and analysis remain off by default
• add a static guard that keeps canonical remediation and built-in hint code free of network, process, cache, and filesystem imports
• document the tested execution and trust boundaries in plain language

Why

Issue 026 needs repeatable evidence for behavior that crosses process and protocol boundaries. These checks make the current plugin, MCP, and read-only remediation guarantees explicit without changing production behavior.

The inventory also found two production concerns that are intentionally excluded from this assurance-only PR: MCP adapter errors are returned verbatim to clients, and subprocess argv logging is not consistently complete or secret-safe. They will be addressed in separate focused fixes before this assurance layer is considered complete.

Validation

• go test ./internal/plugin ./internal/mcp ./internal/remediation ./internal/registry ./test/assurance
• make generate
• make fmt-check
• make lint
• make test
• make build
• git diff --check

[coderabbitai]

Warning

Review limit reached

@bomly-guy, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 40fd8990-70a6-473b-baa4-cc90af65744d

📥 Commits

Reviewing files that changed from the base of the PR and between 508f2d1 and d5c24bb.

📒 Files selected for processing (5)

• internal/mcp/mcp_test.go
• internal/plugin/env_test.go
• internal/plugin/plugin_test.go
• test/assurance/EXECUTION_BOUNDARIES.md
• test/assurance/execution_boundaries_test.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch security-execution-boundaries

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Bomly Diff Summary

Compared 508f2d1b639fc3617a17b0987cc168dcf4df4a76 to d5c24bb63249b8db410cc174421384bf5f2b9e96.

Overview

Status	Manifests	Dependencies	Findings	Duration
✅ Pass	+0 / ~0 / -0	+0 / ~0 / -0	0 introduced / 0 persisted / 0 resolved	1m 18s

Dependency Changes

✅ No dependency changes.

Vulnerabilities

✅ No vulnerability changes.

License Changes

✅ No license changes.

Project Posture

✅ No project posture changes (--matchers +scorecard was not selected).

Policy Findings

✅ No policy differences were identified.

[bomly-guy]

Reviewed. Three worthwhile additions here: TestPluginEnvDoesNotForwardUnrelatedHostEnvironment pins an allowlist rather than a denylist (asserting AWS_SECRET_ACCESS_KEY / GITHUB_TOKEN / DATABASE_URL are absent and that the required protocol values are present, so a future "just forward everything" regression fails loudly); the disabled-plugin check in TestInstallDevBinaryVerifyEnableDisableAndUninstall closes the gap between "recorded disabled" and "actually absent from runtime planning", which are different properties; and the MCP default-authority test feeding control characters and traversal text into path/package/base covers the "doesn't panic and doesn't silently enable" pair together.

The import-boundary test is weaker than it appears. TestReadOnlyRemediationSourcesDoNotImportExecutionOrIOPackages uses parser.ParseFile(..., parser.ImportsOnly) on two named files plus any remediation.go under internal/detectors. That catches only direct, same-file imports. internal/remediation/derive.go can import a sibling package in internal/remediation that imports os/exec, and the test passes. It also can't see the rest of each package: another file in internal/remediation importing net/http is invisible, even though it's the same package and can be called from derive.go freely.

Since the claim in EXECUTION_BOUNDARIES.md is about the package ("the central derivation code ... do not import network, process, cache, or filesystem packages"), the check should be at package granularity and transitive. go list -deps ./internal/remediation and asserting the forbidden set doesn't appear in the closure would actually prove the stated property, and is about the same amount of code. If per-file is intentional (e.g. you want os usable elsewhere in the package but not in derive.go), then the doc should say "these files" rather than "the central derivation code", and the test should enumerate all files in the package rather than two by name.

Also: os is in the forbidden set but the test file itself uses os.DirEntry — fine, but note that forbidding os outright is a fairly blunt proxy for "filesystem access" and will fire on something innocuous like os.Getenv for a feature flag. Worth a comment recording that the strictness is deliberate so the next person doesn't quietly delete the entry.

Documentation ordering. "Assurance for this rule is completed only when every command runner uses the shared redaction boundary" makes the subprocess-logging section explicitly conditional on #334. Good that it's stated rather than implied — please just make sure #334 lands before or with this, and before #337, whose table asserts the completed state.

Formatting nit: the forbidden map mixes short stdlib keys with long module paths, so gofmt aligns the whole block to the longest key and the stdlib entries read oddly. Splitting into two maps (stdlib / internal) or one []struct{path, reason string} would read better. Cosmetic only.

Finally, the "MCP is not a sandbox" and "an enabled external plugin is still native code" paragraphs are the most valuable text in the PR — they're the claims most likely to be over-read by someone evaluating Bomly for a locked-down environment, and stating the limit plainly is the right call.

[bomly-guy]

Re-reviewed at Strengthen execution boundary assurance. The import-boundary test now shells out to go list -deps -json and checks the full transitive closure of the target packages, which is what the doc's package-level claim actually needs — an indirect os/exec reached through a sibling package is now caught, and the "only two named files" blind spot is gone. Failing loudly when go list omits the target package is a good touch; a silently-empty dependency set would have made the test vacuous.

Nothing further from me on this one.