Clarify network and plugin trust boundaries - #324
Conversation
Add a pipeline regression matrix proving that plain, audit-only, analysis-only, and policy-only requests never prepare or invoke matchers. Preserve explicit enrichment and internal matching as the only matcher execution signals. Document remote target cloning and detector subprocesses as network boundaries separate from enrichment. Correct the built-in service inventory and state clearly that enabled native plugins run with the user's privileges rather than inside an operating-system sandbox.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe change clarifies that network-backed matchers require ChangesNetwork intent model
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Bomly Diff SummaryCompared Overview
Dependency Changes✅ No dependency changes. Vulnerabilities✅ No vulnerability changes. License Changes✅ No license changes. Project Posture✅ No project posture changes ( Policy Findings✅ No policy differences were identified. |
bomly-guy
left a comment
There was a problem hiding this comment.
Reviewed the docs sweep and the new pipeline matcher-gate test. The test is well-constructed — I verified it passes and that matcher.Ready in the engine is only reachable through the gated runMatch path, so the guarantee it encodes ("no matcher preparation or execution without explicit intent") is real, not just plausible.
One repo-consistency gap worth fixing in this PR:
CLAUDE.md and AGENTS.md network allowlists are now out of sync with the docs. This PR adds "Grype's vulnerability database distribution service" to the permitted enrichment-time services in dev-docs/ARCHITECTURE.md, but the normative "Network calls (--enrich) permitted only to: …" lists in CLAUDE.md (Non-Negotiables) and AGENTS.md still enumerate only the six HTTP hosts and omit the Grype DB endpoint. Since those lists are worded as exhaustive ("permitted only to"), the built-in Grype matcher's DB download now violates the letter of the repo's own contributor rules — the same kind of imprecision this PR fixes in the user docs. Suggest updating both files in this PR (they're the lists reviewers and agents actually enforce against). Relatedly, those lists still present ClearlyDefined and endoflife.date without the "external plugin" framing the user docs now use — worth aligning while you're in there.
Inline comments cover two smaller accuracy/consistency nits. No blocking issues.
| These are the **only** hosts Bomly's built-in matchers contact during enrichment. No telemetry. No data exfiltration. No credentials sent. External plugin matchers may contact their own documented services after you install and enable them. See [docs/ARCHITECTURE.md](ARCHITECTURE.md) for the full network model. | ||
| The OpenSSF Scorecard matcher also calls `api.scorecard.dev`. Grype manages | ||
| its own vulnerability database and may download or update that database during | ||
| enrichment. Bomly sends no telemetry or credentials to its built-in matcher |
There was a problem hiding this comment.
Nit on precision, since precision is this PR's stated goal: the builtin Grype adapter only downloads the DB when it's absent (needsDownload := !a.dbExists() in internal/matchers/grype/builtin.go, and LoadVulnerabilityDB is called with update=needsDownload), and ValidateAge is disabled — so Bomly never updates an existing DB, it only performs the initial download. "may download or update" overstates the network surface slightly. Consider "downloads its vulnerability database on first use" (here and in the internal/support/component_docs.go template). Being conservative is fine too, but the current wording implies a recurring update check that doesn't exist.
|
|
||
| **Detector network behavior is per-implementation.** Lockfile-parser detectors (npm, pnpm, yarn, Composer, Bundler, NuGet, GitHub Actions, SBOM ingest, …) are pure file parsers and make no network calls. Build-tool primary detectors (`go-detector`, `maven-detector`, `gradle-detector`, `sbt-native-detector`) shell out to the build tool, which may download packages from registries during normal resolution — this is the build tool's behavior, not Bomly's. Hybrid detectors (`cargo`, `poetry`, `uv`) prefer the lockfile and use `--locked`/`--no-sync` flags on the build-tool fallback to stay offline. See [DETECTORS.md → Network behavior](../docs/DETECTORS.md#network-behavior). | ||
|
|
||
| **Target materialization is a separate network boundary.** `--url` explicitly |
There was a problem hiding this comment.
Just above this hunk, line 395 still opens with "Matchers are offline-safe by default." — the exact phrase this PR retires everywhere else (and whose removal generate_test.go now asserts for the generated matcher overview). It's technically scoped to matchers so it isn't wrong, but leaving the retired slogan in the contributor deep-dive undercuts the sweep. Suggest rewording it to match the new "network-backed matchers are off by default" framing.
| @@ -66,8 +66,8 @@ This check is name-based and needs no enrichment, so it runs fully offline. See | |||
| **Goal:** analyze dependencies with zero outbound network calls. | |||
There was a problem hiding this comment.
Minor: the goal line still promises "zero outbound network calls" while the command comment right below it was reworded to "detector behavior varies" precisely because that promise doesn't always hold. Since the recipe's caveat paragraph already explains the detector exception, consider softening the goal to something like "analyze dependencies without contacting enrichment services" so the section doesn't open with the claim it then walks back.
Describe bundled Grype as downloading its database on first use and distinguish the lite build's external database behavior. Align contributor network rules with built-in and external matcher boundaries, retire the remaining broad offline slogan, and make the offline use-case goal consistent with detector network caveats.
What changed
Why
The matcher gate already enforced explicit intent, but the repository lacked one test covering all neighboring pipeline options. Several public pages also used broad offline and sandbox wording that did not account for remote target cloning, build-tool subprocesses, or native plugin privileges.
This PR makes the tested guarantee narrow and accurate: network-backed matchers require explicit matching intent. Other network and host-access boundaries are documented independently.
User impact
There is no command or output contract change. Users get clearer guidance about when a scan may use the network and what enabling a native plugin authorizes.
Validation
make generatemake fmt-checkmake lintmake testmake buildgit diff --checkSummary by CodeRabbit
Documentation
Tests