-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: extract analyzers into component modules #386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b46bed6
c972acf
b359013
23392a5
57b0efc
1fb840e
e53a546
9a6be85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,9 +27,11 @@ jobs: | |
| cache: true | ||
| cache-dependency-path: go.sum | ||
| - name: Run golangci-lint | ||
| uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 | ||
| with: | ||
| version: v2.12.0 | ||
| # make lint installs the Makefile-pinned golangci-lint and runs it | ||
| # for the root module and every nested component module; the | ||
| # golangci-lint-action's bare root invocation never descends into | ||
| # components/. | ||
| run: make lint | ||
|
|
||
| test: | ||
| name: Test | ||
|
|
@@ -63,33 +65,6 @@ jobs: | |
| - name: Build lite Bomly binary | ||
| run: go build -tags "bomly_external_syft,bomly_external_grype" -o /tmp/bomly-lite ./cmd/bomly | ||
|
|
||
| pinned-build: | ||
| name: Pinned build (workspace off) | ||
| # Component-module pseudo-versions only resolve after a wave PR merges, | ||
| # so this pins-only guard runs on pushes to main rather than on PRs. | ||
| if: github.event_name == 'push' | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| GOWORK: off | ||
| steps: | ||
| - name: Check out repository | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| - name: Set up Go | ||
| uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 | ||
| with: | ||
| go-version-file: go.mod | ||
| cache: true | ||
| cache-dependency-path: go.sum | ||
| - name: Build with module pins only | ||
| run: go build ./cmd/bomly | ||
| - name: Build lite variant with module pins only | ||
| # Mirrors the regular build job's lite variant so a stale pin in a | ||
| # future extracted Syft/Grype component module fails here instead of | ||
| # only in GoReleaser. | ||
| run: go build -tags "bomly_external_syft,bomly_external_grype" ./cmd/bomly | ||
| - name: Test with module pins only | ||
| run: go test ./internal/... ./cmd/... | ||
|
|
||
| format: | ||
| name: Format | ||
| runs-on: ubuntu-latest | ||
|
|
@@ -123,14 +98,40 @@ jobs: | |
| set -euo pipefail | ||
| go mod tidy | ||
| git diff --exit-code -- go.mod go.sum | ||
| - name: Forbid replace directives | ||
| - name: Check component module metadata drift | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| for modfile in components/*/*/go.mod; do | ||
| [ -e "$modfile" ] || continue | ||
| dir="$(dirname "$modfile")" | ||
| (cd "$dir" && go mod tidy) | ||
| done | ||
| git diff --exit-code -- components/ | ||
| - name: Enforce the replace-directive allowlist | ||
| shell: bash | ||
| # The root module intentionally replaces each in-repo component | ||
| # module with its ./components/ directory (documented trade-off: | ||
| # remote `go install ...@latest` is unsupported; install from | ||
| # releases or a clone). Any other replace target in the root, and | ||
| # any replace at all in a component module, is forbidden. | ||
| # `go mod edit -json` sees single-line and block-form directives | ||
| # alike, which a line-oriented grep would miss. | ||
| run: | | ||
| set -euo pipefail | ||
| if grep -E '^replace ' go.mod; then | ||
| echo "go.mod must not contain replace directives" >&2 | ||
| bad="$(go mod edit -json | jq -r '.Replace[]? | select(.New.Path | startswith("./components/") | not) | .Old.Path')" | ||
| if [ -n "$bad" ]; then | ||
| echo "root go.mod may only replace in-repo ./components/ modules; found: $bad" >&2 | ||
| exit 1 | ||
| fi | ||
|
Comment on lines
+122
to
126
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow excerpt ---'
sed -n '95,140p' .github/workflows/ci.yml
printf '%s\n' '--- module files ---'
git ls-files 'go.mod' 'components/**/go.mod' | sort | while IFS= read -r f; do
printf '\n[%s]\n' "$f"
sed -n '1,120p' "$f"
done
printf '%s\n' '--- replacement-related references ---'
rg -n --glob '*.yml' --glob '*.yaml' --glob 'go.mod' --glob '*.mk' 'go mod edit|replace|components/' .Repository: bomly-dev/bomly-cli Length of output: 13642 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
workflow = Path(".github/workflows/ci.yml").read_text()
block = workflow[workflow.find('bad="$(go mod edit -json'):workflow.find('bad="$(go mod edit -json') + 500]
print("--- validator logic ---")
print(block)
print("--- parsed root replacements ---")
root = Path("go.mod").read_text()
for line in root.splitlines():
if re.match(r"\s*replace\b", line) or (line.strip() and "=>" in line):
print(line)
PYRepository: bomly-dev/bomly-cli Length of output: 1133 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
def parse_replace_lines(text):
out = []
for raw in text.splitlines():
line = raw.split("//", 1)[0].strip()
if not line or line.startswith("module ") or line.startswith("require "):
continue
if "=>" in line:
lhs, rhs = map(str.strip, line.split("=>", 1))
old = lhs.split()[0]
new = rhs.split()[0]
out.append((old, new))
return out
print("--- all replace directives ---")
for path in [Path("go.mod"), *sorted(Path("components").glob("*/go.mod"))]:
if not path.exists():
continue
print(f"[{path}]")
for old, new in parse_replace_lines(path.read_text()):
print(f"old={old!r} new={new!r}")
print("--- module declarations ---")
for path in [Path("go.mod"), *sorted(Path("components").glob("*/go.mod"))]:
if path.exists():
m = re.search(r"(?m)^module\s+(\S+)", path.read_text())
print(f"{path}: {m.group(1) if m else '<missing>'}")
PYRepository: bomly-dev/bomly-cli Length of output: 473 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
workflow = Path(".github/workflows/ci.yml").read_text()
start = workflow.index('bad="$(go mod edit -json')
end = workflow.index('fi', start) + 2
print(workflow[start:end])
# Model the exact jq predicate for representative replace directives.
cases = [
("example.com/acme/third-party", "./components/analyzers/pyreach"),
("example.com/repo/components/analyzers/pyreach", "./components/analyzers/pyreach"),
("example.com/repo/components/other", "./components/analyzers/pyreach"),
("example.com/repo/components/analyzers/pyreach", "./components/analyzers/pyreach"),
]
print("--- predicate results ---")
for old, new in cases:
accepted = new.startswith("./components/")
print(f"{old} => {new}: {'accepted' if accepted else 'rejected'}")
PYRepository: bomly-dev/bomly-cli Length of output: 791 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- .github/workflows/ci.yml lines 115-130 ---'
nl -ba .github/workflows/ci.yml | sed -n '115,130p'
printf '%s\n' '--- module declarations and root replacements ---'
python3 - <<'PY'
from pathlib import Path
import re
for path in [Path("go.mod"), *sorted(Path("components").glob("*/go.mod"))]:
if not path.exists():
continue
text = path.read_text()
module = re.search(r"(?m)^module\s+(\S+)", text)
print(f"{path}: module={module.group(1) if module else None}")
for line in text.splitlines():
if "replace" in line or "=>" in line:
print(f" {line}")
PYRepository: bomly-dev/bomly-cli Length of output: 762 Restrict root The allowlist accepts any 🤖 Prompt for AI Agents |
||
| for modfile in components/*/*/go.mod; do | ||
| [ -e "$modfile" ] || continue | ||
| count="$(go mod edit -json "$modfile" | jq '.Replace | length')" | ||
| if [ "$count" != "0" ] && [ "$count" != "null" ]; then | ||
| echo "$modfile must not contain replace directives" >&2 | ||
| exit 1 | ||
| fi | ||
| done | ||
|
|
||
| generated-docs: | ||
| name: Generated docs drift | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,7 @@ make generate # regenerate config reference, JSON schemas, schema doc | |
| Always run `make test` after changes. All tests must pass before marking work is done. | ||
| If you change `internal/cli/config.go`, `internal/output/*`, or `internal/registry/support.go`, or bump the pinned `bomly-dev/bomly-sdk` version (its catalog or support-matrix data feeds the generated docs), also run `make generate` and commit the docs drift. | ||
|
|
||
| `go.mod` pins released versions and must not contain `replace` directives on main (CI enforces this). The committed `go.work` lists in-repo modules only (root now; `components/*` as waves land). Local cross-repo SDK development: `go work use ../bomly-sdk` (never commit that entry). | ||
| `go.mod` pins released versions. The only `replace` directives allowed on main point in-repo component modules at their `./components/<kind>/<name>` directories (CI enforces this allowlist; component go.mods carry no replaces at all). Trade-off, accepted deliberately: remote `go install ...@latest` is unsupported for replace-carrying modules — users install from releases, package managers, or a clone. Local cross-repo SDK development: `go work init . ../bomly-sdk` (never commit `go.work`). | ||
|
|
||
| ### Git Worktrees | ||
|
|
||
|
|
@@ -50,7 +50,7 @@ See [`dev-docs/ARCHITECTURE.md`](dev-docs/ARCHITECTURE.md) for full detail (the | |
| | `internal/detectors/*` | Concrete dependency resolution per ecosystem (gomod, gradle, maven, node, python, sbom, syft) | | ||
| | `internal/matchers/*` | External enrichment matchers (osv, grype, deps.dev, scorecard; ClearlyDefined and eol run as external matcher plugins); the shared cache lives in `bomly-sdk/filecache` | | ||
| | `internal/auditors/*` | Policy evaluators and audit-only logic (policy, noop) | | ||
| | `internal/analyzers/*` | Built-in reachability analyzers (govulncheck, jsreach) | | ||
| | `components/analyzers/*` | Built-in reachability analyzers as nested component modules (govulncheck, jsreach, pyreach, jvmreach) | | ||
| | `internal/baseline` | Portable package-finding baseline codec and audit-integrated policy-status resolver | | ||
| | `internal/remediation` | Canonical vulnerability fix status, version, detector-hint validation, and occurrence suggestions | | ||
| | `internal/sbom` | SBOM codec (SPDX 2.3, CycloneDX) | | ||
|
|
@@ -82,14 +82,14 @@ Runtime preparation is owned by `internal/engine`: build the filtered registry o | |
|
|
||
| - Shared helper code (bounded filesystem/subprocess ops, file cache, subprocess logging, detector/matcher helpers, test kit) lives in `bomly-sdk` subpackages: `system`, `filecache`, `logkit`, `detectorkit`, `matcherkit`, `testkit`. Do not reintroduce CLI-internal copies. | ||
| - Extracted components will live under `components/<kind>/<name>/` as separate Go modules with their own `go.mod`, tagged per module as `components/<kind>/<name>/vX.Y.Z`. | ||
| - The committed `go.work` puts the repo in workspace mode for local development; waves add `use ./components/...` entries. Release and pinned builds run with `GOWORK=off` (GoReleaser sets it explicitly; CI's `pinned-build` job verifies the module pins alone still build on pushes to `main`). | ||
| - Component modules under `components/` are consumed through root `go.mod` `require` + directory `replace` entries, so every build — local, CI, and GoReleaser — resolves them from the checkout itself. There is no committed workspace file and no separate pinned-build guard; the regular build and test jobs cover the only resolution mode that exists. | ||
| - Each extraction wave lands as **one atomic PR**: move the code into its component module, add the `use` entry, and keep the root module compiling in the same change. | ||
| - `scripts/release-components.sh` (also `make release-components`) is the release train: component modules version in lockstep with the CLI — after a CLI release tag exists, the script tags every component module at that same version (idempotent; unchanged modules get empty releases by design); `--apply` (ARGS="--apply") creates and pushes the tags and prints the root `go get` pin bumps for the follow-up PR. | ||
|
Comment on lines
+85
to
87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Replace the stale workspace instruction with root module wiring. The component model uses root
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| ### Package Boundaries | ||
|
|
||
| - `internal/detectors/*` must not import `internal/engine` or `internal/registry`. Concrete detectors may depend on `internal/detectors` (name constants), the SDK and its helper subpackages (`system` for bounded filesystem and subprocess operations, `detectorkit` for shared detector helpers), and local helpers. | ||
| - Built-in analyzers may depend on the SDK and its helper subpackages (`system` for bounded filesystem and subprocess operations, `filecache`, `logkit`), and local helpers. They must not import `internal/engine` or `internal/registry`. | ||
| - Built-in analyzers live in nested component modules under `components/analyzers/<name>/` (own `go.mod`, consumed by `internal/composition` via the root `go.mod` require + directory replace pair). They may depend on the SDK and its helper subpackages (`system` for bounded filesystem and subprocess operations, `filecache`, `logkit`), and local helpers. They must not import any `internal/*` package. | ||
| - `internal/detectors` owns detector-facing contracts such as `Detector`, `DetectorDescriptor`, `ResolveGraphRequest`, and detector helper functions. | ||
| - The SDK owns neutral shared identifiers and support metadata that would otherwise create package cycles, including ecosystems, package managers, detector types, and support-matrix data. | ||
| - `internal/baseline` owns the baseline document and matching implementation. It depends on the SDK policy contracts and must not be imported by `internal/engine`. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prefix check accepts replacements that are not the permitted component-module mappings, such as redirecting
github.com/bomly-dev/bomly-sdkto./components/analyzers/govulncheckor using a path like./components/../other; both satisfystartswith("./components/"). Consequently this job can report that the allowlist is enforced while an arbitrary module or normalized path is substituted. Compare each old module path and cleaned target directory against the module declarations discovered undercomponents/*/*/go.mod.AGENTS.md reference: AGENTS.md:L30-L30
Useful? React with 👍 / 👎.