Skip to content

Commit d1d5671

Browse files
authored
Add execution boundary assurance (#328)
* test: add execution boundary assurance Verify that managed plugins receive only selected runtime settings and remain disabled until explicitly enabled. Exercise MCP requests with hostile text while optional network, audit, and analysis work remains off by default. Add a static guard and assurance record for the read-only remediation boundary. * Strengthen execution boundary assurance
1 parent d2447c9 commit d1d5671

5 files changed

Lines changed: 272 additions & 0 deletions

File tree

internal/mcp/mcp_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,63 @@ func TestScanTool_PropagatesScope(t *testing.T) {
183183
}
184184
}
185185

186+
func TestToolsDoNotEnableNetworkOrAnalysisByDefault(t *testing.T) {
187+
t.Run("scan", func(t *testing.T) {
188+
adapter := &mockAdapter{
189+
scanResult: mcp.ScanRunResult{
190+
Response: output.ScanResponse{Command: "scan"},
191+
},
192+
}
193+
c := newTestClient(t, adapter)
194+
result := callTool(t, c, "bomly_scan", map[string]any{
195+
"path": "../../untrusted\npath",
196+
})
197+
if result.IsError {
198+
t.Fatalf("unexpected tool error: %v", result.Content)
199+
}
200+
if adapter.scanReq.Enrich || adapter.scanReq.Audit || adapter.scanReq.Analyze {
201+
t.Fatalf("scan enabled optional work without permission: %#v", adapter.scanReq)
202+
}
203+
})
204+
205+
t.Run("explain", func(t *testing.T) {
206+
adapter := &mockAdapter{
207+
explainResult: mcp.ExplainRunResult{
208+
Response: output.ExplainResponse{Command: "explain"},
209+
},
210+
}
211+
c := newTestClient(t, adapter)
212+
result := callTool(t, c, "bomly_explain", map[string]any{
213+
"package": "pkg:npm/example@1.0.0\nuntrusted",
214+
})
215+
if result.IsError {
216+
t.Fatalf("unexpected tool error: %v", result.Content)
217+
}
218+
if adapter.explainReq.Enrich || adapter.explainReq.Audit || adapter.explainReq.Analyze {
219+
t.Fatalf("explain enabled optional work without permission: %#v", adapter.explainReq)
220+
}
221+
})
222+
223+
t.Run("diff", func(t *testing.T) {
224+
adapter := &mockAdapter{
225+
diffResult: mcp.DiffRunResult{
226+
Response: output.DiffResponse{Command: "diff"},
227+
},
228+
}
229+
c := newTestClient(t, adapter)
230+
result := callTool(t, c, "bomly_diff", map[string]any{
231+
"base": "main\nuntrusted",
232+
"head": "HEAD",
233+
})
234+
if result.IsError {
235+
t.Fatalf("unexpected tool error: %v", result.Content)
236+
}
237+
if adapter.diffReq.Enrich || adapter.diffReq.Audit || adapter.diffReq.Analyze {
238+
t.Fatalf("diff enabled optional work without permission: %#v", adapter.diffReq)
239+
}
240+
})
241+
}
242+
186243
func TestScanTool_PropagatesPolicyArguments(t *testing.T) {
187244
adapter := &mockAdapter{scanResult: mcp.ScanRunResult{Response: output.ScanResponse{Command: "scan"}}}
188245
c := newTestClient(t, adapter)

internal/plugin/env_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,35 @@ func TestPluginEnvForwardsStandardProxyEnvWhenBomlyProxyUnset(t *testing.T) {
9292
}
9393
}
9494

95+
func TestPluginEnvDoesNotForwardUnrelatedHostEnvironment(t *testing.T) {
96+
t.Setenv("AWS_SECRET_ACCESS_KEY", "ambient-cloud-secret")
97+
t.Setenv("GITHUB_TOKEN", "ambient-github-secret")
98+
t.Setenv("DATABASE_URL", "postgres://user:ambient-db-secret@example.com/db")
99+
t.Setenv("BOMLY_OSV_API_BASE", "https://unrelated.example.test")
100+
101+
env, cleanup, err := pluginEnv(LaunchOptions{}, "acme.matcher")
102+
if err != nil {
103+
t.Fatalf("pluginEnv() error = %v", err)
104+
}
105+
defer cleanup()
106+
107+
values := envMap(env)
108+
for _, name := range []string{
109+
"AWS_SECRET_ACCESS_KEY",
110+
"GITHUB_TOKEN",
111+
"DATABASE_URL",
112+
"BOMLY_OSV_API_BASE",
113+
} {
114+
if value, ok := values[name]; ok {
115+
t.Fatalf("plugin environment forwarded unrelated %s=%q", name, value)
116+
}
117+
}
118+
if values[EnvPluginAPIVersion] != sdk.PluginAPIVersion ||
119+
values[sdk.EnvPluginID] != "acme.matcher" {
120+
t.Fatalf("plugin environment omitted required protocol values: %#v", values)
121+
}
122+
}
123+
95124
func TestPluginEnvOnlyWritesSelectedPluginConfig(t *testing.T) {
96125
env, cleanup, err := pluginEnv(LaunchOptions{
97126
PluginConfigs: map[string]map[string]any{

internal/plugin/plugin_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ func TestInstallDevBinaryVerifyEnableDisableAndUninstall(t *testing.T) {
3434
if result.Installed.Enabled {
3535
t.Fatalf("expected plugin install to record disabled state by default")
3636
}
37+
disabledRegistry := engine.NewRegistry(engine.RegistryConfigs{}, *zap.NewNop())
38+
disabledRegistry.Build()
39+
if err := managedplugin.RegisterRuntimePlugins(context.Background(), disabledRegistry, root); err != nil {
40+
t.Fatalf("RegisterRuntimePlugins() with disabled plugin error = %v", err)
41+
}
42+
for _, detector := range disabledRegistry.AllDetectors() {
43+
if detector.Descriptor().Name == "acme.detector.fake" {
44+
t.Fatal("disabled external plugin joined the runtime registry")
45+
}
46+
}
3747
manifestBytes, err := os.ReadFile(filepath.Join(result.Installed.Path, "bomly-plugin.json"))
3848
if err != nil {
3949
t.Fatalf("read installed manifest: %v", err)
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Execution Boundary Assurance
2+
3+
Bomly can start package managers and enabled native plugins. MCP can request
4+
the same operations as the CLI. These tests separate the controls Bomly
5+
enforces from authority delegated to a program the user selected.
6+
7+
| Boundary | Regression evidence | What the evidence proves |
8+
| --- | --- | --- |
9+
| Managed plugin environment | `TestPluginEnvIncludesProxyAndPluginConfig`, `TestPluginEnvForwardsStandardProxyEnvWhenBomlyProxyUnset`, `TestPluginEnvDoesNotForwardUnrelatedHostEnvironment`, `TestPluginEnvOnlyWritesSelectedPluginConfig` | Managed plugins receive protocol identity, selected plugin config, and configured or standard proxy settings. Unrelated host values such as cloud tokens and database URLs are not copied. |
10+
| Plugin lifecycle | `TestInstallDevBinaryVerifyEnableDisableAndUninstall`, `TestPrepareLoadsAndRunsExternalDetector`, `TestExternalMatcherReceivesAndReturnsRegistry` | Installation leaves a plugin disabled. Disabled plugins do not join runtime planning; an explicitly enabled plugin can run through its advertised contract. |
11+
| Protocol and fallback behavior | `TestProtocolV1DetectorSnapshotDefaultsAbsentOptionalCapabilities`, `TestRuntimeSnapshotRejectsUnadvertisedOrMalformedRole`, `TestResolveDetectors_FallbackAnnotatesResult`, `TestPipeline_RunRecordsFallbackWarning` | Older plugins work without optional capabilities. Invalid roles fail, and detector failure uses the normal fallback path. |
12+
| MCP default authority | `TestToolsDoNotEnableNetworkOrAnalysisByDefault` | Scan, explain, and diff requests do not enable enrichment, audit, or analysis unless their own fields request it. Hostile path, package, and Git text does not panic or silently grant those permissions. |
13+
| MCP response size | `TestCompactScanInventoryCapIsDeterministicAndCounted`, `TestCompactScanCapsDiagnosticsWithVisibleMarker`, `TestCompactRemediationCapsAliasesAndFindingsWithCounters`, `TestCompactScanSizeStaysUnderBudget` | Compact results stay within configured collection caps and report omitted data. |
14+
| Central remediation dependencies | `TestRemediationPackageDependencyBoundaries/central_derivation` | `go list -deps -json` checks the complete `internal/remediation` package and its transitive Bomly package graph. It cannot directly import network, OS, process, system, or cache packages, and cannot reach Git, plugins, system execution, or matcher caches transitively. The direct `os` prohibition is deliberately strict: even environment reads are rejected so this policy package cannot quietly gain ambient host authority. |
15+
| Detector hint dependencies | `TestRemediationPackageDependencyBoundaries/detector_hint_packages` | Each package that owns built-in hints is checked at package granularity. Hint-owning detector packages cannot directly import networking, Git, matcher cache, plugin, or central remediation packages and cannot reach Bomly's Git, cache, plugin, or policy packages transitively. Detector packages legitimately retain `internal/system` and `os/exec` for their separate graph-resolution role. |
16+
| Remediation data contract | `TestExternalDetectorProvidesAdvertisedRemediationHints`, `TestDerivePackageRemediation`, `TestDerivePackageRemediationsOverwritesAndIsIdempotent`, `TestValidateHintsSanitizesAndBoundsAdvice`, `TestCollectHintsBoundsAndSanitizesDiagnostics`, `TestDeriveRejectsUnadvertisedAndUnknownHints` | Core passes cloned data, validates occurrence references and advertised strategies, bounds provider text, and chooses status, version, and action centrally. Returned hints cannot authorize writes or execution. |
17+
| Subprocess diagnostics | `TestSanitizeArgsRedactsCredentialValuesAndURLUserinfo`, `TestSanitizeArgsDoesNotTreatOrdinaryAuthoredFlagsAsCredentials`, `TestNewConsoleAndCommandStderr`, `TestCommandStderrNilAndHidden`, `TestInstallLogsReproducibleCommandWithoutCredentials` in PR #334 | Debug logs retain executable, credential-sanitized arguments, and working directory. Arbitrary subprocess stderr is counted but not retained or mirrored. |
18+
19+
## Residual Authority
20+
21+
MCP is not a sandbox. A requested path, Git URL, image, plugin, or
22+
package-manager operation has the same authority it has in the CLI.
23+
24+
An enabled external plugin is a native process with the user's privileges. The
25+
protocol constrains data Bomly accepts from it; it cannot prevent that process
26+
from reading files, writing files, using the network, or starting another
27+
program.
28+
29+
Detector packages combine read-only hint methods with package-manager graph
30+
resolution. Their package dependency graphs therefore include process and
31+
filesystem helpers by design. The SDK provider contract, cloned requests, and
32+
core validation enforce hint behavior inside Bomly; they are architecture
33+
controls, not an operating-system sandbox for enabled native plugins.
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package assurance
2+
3+
import (
4+
"bytes"
5+
"encoding/json"
6+
"errors"
7+
"io"
8+
"os/exec"
9+
"path/filepath"
10+
"runtime"
11+
"strings"
12+
"testing"
13+
)
14+
15+
type listedPackage struct {
16+
ImportPath string
17+
Imports []string
18+
}
19+
20+
func TestRemediationPackageDependencyBoundaries(t *testing.T) {
21+
root := repositoryRoot(t)
22+
const module = "github.com/bomly-dev/bomly-cli/"
23+
24+
t.Run("central derivation", func(t *testing.T) {
25+
packages := goListDependencies(t, root, "./internal/remediation")
26+
assertDirectImportsAbsent(t, packages, module+"internal/remediation", map[string]string{
27+
"net": "network access",
28+
"net/http": "HTTP access",
29+
"os": "filesystem or process environment access",
30+
"os/exec": "subprocess execution",
31+
module + "internal/system": "filesystem or subprocess access",
32+
module + "internal/matchers/cache": "cache filesystem access",
33+
})
34+
assertDependenciesAbsent(t, packages, map[string]string{
35+
module + "internal/git": "Git filesystem or subprocess access",
36+
module + "internal/matchers/cache": "cache filesystem access",
37+
module + "internal/plugin": "native plugin execution",
38+
module + "internal/system": "filesystem or subprocess access",
39+
})
40+
})
41+
42+
// These packages also own detector resolution, so their complete dependency
43+
// graphs legitimately include internal/system and os/exec. At package
44+
// granularity, enforce that remediation hints do not acquire network,
45+
// cache, plugin, Git, or central-policy dependencies. Request immutability
46+
// and read-only provider behavior are covered by the remediation contract
47+
// tests named in EXECUTION_BOUNDARIES.md.
48+
detectorHintPackages := []struct {
49+
name string
50+
path string
51+
}{
52+
{name: "shared", path: "./internal/detectors"},
53+
{name: "cargo", path: "./internal/detectors/cargo"},
54+
{name: "composer", path: "./internal/detectors/composer"},
55+
{name: "gomod", path: "./internal/detectors/gomod"},
56+
{name: "gradle", path: "./internal/detectors/gradle"},
57+
{name: "maven", path: "./internal/detectors/maven"},
58+
{name: "bun", path: "./internal/detectors/node/bun"},
59+
{name: "npm", path: "./internal/detectors/node/npm"},
60+
{name: "pnpm", path: "./internal/detectors/node/pnpm"},
61+
{name: "yarn", path: "./internal/detectors/node/yarn"},
62+
{name: "python", path: "./internal/detectors/python"},
63+
{name: "ruby", path: "./internal/detectors/ruby"},
64+
}
65+
t.Run("detector hint packages", func(t *testing.T) {
66+
for _, target := range detectorHintPackages {
67+
target := target
68+
t.Run(target.name, func(t *testing.T) {
69+
packages := goListDependencies(t, root, target.path)
70+
importPath := module + strings.TrimPrefix(target.path, "./")
71+
assertDirectImportsAbsent(t, packages, importPath, map[string]string{
72+
"net": "network access",
73+
"net/http": "HTTP access",
74+
module + "internal/git": "Git access",
75+
module + "internal/matchers/cache": "cache filesystem access",
76+
module + "internal/plugin": "native plugin execution",
77+
module + "internal/remediation": "central remediation policy",
78+
})
79+
assertDependenciesAbsent(t, packages, map[string]string{
80+
module + "internal/git": "Git access",
81+
module + "internal/matchers/cache": "cache filesystem access",
82+
module + "internal/plugin": "native plugin execution",
83+
module + "internal/remediation": "central remediation policy",
84+
})
85+
})
86+
}
87+
})
88+
}
89+
90+
func goListDependencies(t *testing.T, root, packagePath string) map[string]listedPackage {
91+
t.Helper()
92+
cmd := exec.Command("go", "list", "-deps", "-json", packagePath)
93+
cmd.Dir = root
94+
output, err := cmd.Output()
95+
if err != nil {
96+
t.Fatalf("go list dependencies for %s: %v", packagePath, err)
97+
}
98+
decoder := json.NewDecoder(bytes.NewReader(output))
99+
packages := map[string]listedPackage{}
100+
for {
101+
var listed listedPackage
102+
err := decoder.Decode(&listed)
103+
if errors.Is(err, io.EOF) {
104+
break
105+
}
106+
if err != nil {
107+
t.Fatalf("decode go list output for %s: %v", packagePath, err)
108+
}
109+
packages[listed.ImportPath] = listed
110+
}
111+
return packages
112+
}
113+
114+
func assertDirectImportsAbsent(t *testing.T, packages map[string]listedPackage, importPath string, forbidden map[string]string) {
115+
t.Helper()
116+
target, ok := packages[importPath]
117+
if !ok {
118+
t.Fatalf("go list omitted target package %q", importPath)
119+
}
120+
for _, imported := range target.Imports {
121+
if reason, found := forbidden[imported]; found {
122+
t.Errorf("package %q directly imports %q, which permits %s", importPath, imported, reason)
123+
}
124+
}
125+
}
126+
127+
func assertDependenciesAbsent(t *testing.T, packages map[string]listedPackage, forbidden map[string]string) {
128+
t.Helper()
129+
for importPath, reason := range forbidden {
130+
if _, found := packages[importPath]; found {
131+
t.Errorf("transitive package graph includes %q, which permits %s", importPath, reason)
132+
}
133+
}
134+
}
135+
136+
func repositoryRoot(t *testing.T) string {
137+
t.Helper()
138+
_, source, _, ok := runtime.Caller(0)
139+
if !ok {
140+
t.Fatal("resolve assurance test source")
141+
}
142+
return filepath.Clean(filepath.Join(filepath.Dir(source), "..", ".."))
143+
}

0 commit comments

Comments
 (0)