Skip to content

fix(001): ecosystem-scope SDK watchlist + drop contradictory unknown-posture notice (spec 210 verify)#13

Merged
bartekus merged 2 commits into
mainfrom
210-verify-followups
Jul 9, 2026
Merged

fix(001): ecosystem-scope SDK watchlist + drop contradictory unknown-posture notice (spec 210 verify)#13
bartekus merged 2 commits into
mainfrom
210-verify-followups

Conversation

@bartekus

@bartekus bartekus commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What

Two verify-verdict fixes + one robustness fix for the spec 210 agentic-posture
verifier, surfaced by AI review of PR #12.

HIGH: ecosystem-scope the SDK watchlist match

purl_matches stripped pkg:<type>/ but ignored the type, so a
pkg:pypi/openai@1.0.0 or pkg:cargo/ai@0.1.0 matched the npm openai / ai
watchlist entries: a cross-ecosystem false positive that would Contradict a
none posture on a mixed-language BOM. The match now requires the purl <type>
to equal the entry's ecosystem (the field was present but dead_code). When a
component has a purl, the match gates on it; only a purl-less component falls
back to a name-only match.

MEDIUM: drop the contradictory unknown-posture notice

An unknown posture string (e.g. "maybe") is an internal-consistency error, but
adjudicate_agentic_posture short-circuited on posture != "none" and returned
Declared, so the same binding got BOTH the error and a reassuring "agentic
posture: DECLARED (agency acknowledged)" notice. append_posture_binding_findings
now skips the cross-check when the binding is internally inconsistent: the error
is authoritative.

LOW: depth-bound the BOM component walk

--sbom-dir is caller-supplied (untrusted), so the recursive components[] walk
is now bounded at MAX_BOM_DEPTH = 64. Real CycloneDX nesting is shallow, so no
legitimate BOM is truncated; a pathologically nested one stops instead of
overflowing the stack.

Verification

  • New tests: purl_matcher_gates_on_ecosystem (pypi/cargo openai/ai do not
    match npm entries; case-insensitive type), posture_findings_unknown_posture_errors_without_declared_notice.
  • 108 lib tests pass; the existing purl_matcher_anchors_on_the_name_segment
    updated to the 3-arg signature.
  • spec-spine compile / lint --fail-on-warn / index check clean; spec 001
    §3 documents the ecosystem-scoping + the inconsistency short-circuit.

Not changed (reviewed, deliberately out of scope)

  • load_agentic_sdk_watchlist expect panic on a malformed embedded file: a
    build-time invariant (committed + tested), consistent with an embedded-asset
    contract; left as-is.
  • _comment key tolerance and the per-call re-parse (a OnceLock micro-opt):
    cosmetic / immaterial for a one-shot CLI.

…unknown-posture notice (spec 210)

Two verify-verdict fixes surfaced by review of PR #12:

- purl_matches gates on the watchlist entry's ecosystem: a same-named dependency
  from another ecosystem (pkg:pypi/openai, pkg:cargo/ai) no longer false-matches
  the npm openai/ai entries on a mixed-language BOM. The ecosystem field
  (previously dead_code) is now load-bearing; a purl-less component keeps the
  name-only fallback.
- append_posture_binding_findings skips the SBOM cross-check when the binding is
  internally inconsistent (e.g. an unknown posture string), so a "maybe" posture
  no longer emits BOTH an error AND a reassuring "DECLARED" notice.

Also depth-bounds the recursive BOM component walk (untrusted --sbom-dir input)
at MAX_BOM_DEPTH so a pathologically nested BOM stops instead of overflowing the
stack.

Tests: purl_matcher_gates_on_ecosystem + posture_findings_unknown_posture_errors_without_declared_notice; 108 lib tests pass. spec 001 §3 documents the ecosystem-scoping + the inconsistency short-circuit.
@bartekus
bartekus enabled auto-merge (squash) July 8, 2026 23:21
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

AI Code Review

Review

Bugs and Logic Errors

Name-only fallback is ecosystem-agnostic (certificate.rs, first_watchlist_match_in_bom):

None => name == Some(entry.name.as_str()),

When a BOM component carries no purl, this matches only on name — no ecosystem gate. A hand-authored BOM with a name: openai Python component would trigger the npm openai watchlist entry. The spec (and the purl path) state the watchlist is ecosystem-scoped, but the fallback path silently bypasses that invariant. The comment calls it "best-effort", but the spec doesn't carve out a best-effort exception: if the intent is to be permissive here, the spec should say so explicitly.

MAX_BOM_DEPTH off-by-one (certificate.rs:1340):

const MAX_BOM_DEPTH: usize = 64;
if depth > MAX_BOM_DEPTH { return None; }

depth > 64 admits depth 64, making the real bound 65, not 64. The constant name and the comment ("stops at MAX_BOM_DEPTH") are misleading. Use depth >= MAX_BOM_DEPTH or rename the constant to MAX_BOM_DEPTH_INCLUSIVE. Not a functional risk at these sizes, but a correctness mismatch.

Security

No new OWASP-class vulnerabilities introduced. The depth-bound on the recursive BOM walk is the right fix for the untrusted-input concern and is correctly placed. eq_ignore_ascii_case for the type comparison is appropriate (purl spec allows case-insensitive type).

Spec-Spine Compliance

  • certificate.rs and specs/001-certificate-verify-core/spec.md change together — coupling gate satisfied.
  • .derived/ shard hashes are regenerated for both by-spec/001-certificate-verify-core.json shards — determinism invariant maintained.
  • The spec update documents both the ecosystem-scoping fix and the early-return-on-inconsistency behaviour. However, the spec says "a purl match requires the BOM component's purl <type> to equal the entry's ecosystem" — the phrase "a purl match requires" implicitly leaves the no-purl fallback unspecified. The spec should either explicitly describe the fallback (best-effort name match, ecosystem-unscoped) or prohibit it.

Performance

None of the changes introduce a performance regression. load_agentic_sdk_watchlist() is called on every BOM check (pre-existing), not new here.

Summary

Two actionable items before merge:

  1. Either ecosystem-gate the name-only fallback (preferred for correctness), or add an explicit spec clause describing the fallback's intentionally weaker guarantee.
  2. Fix the depth > MAX_BOM_DEPTH bound to >=, or rename the constant to match the actual semantics.

Automated review by Claude. Diff size: 100 lines.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

Code Review

Bugs and Logic Errors

Purl-present suppresses name fallback (intentional but undocumented edge case)

In the refactored walk loop (first_watchlist_match_in_bom):

let matched = match purl {
    Some(p) => purl_matches(p, &entry.name, &entry.ecosystem),
    None => name == Some(entry.name.as_str()),
};

When a component carries a purl from a different ecosystem (e.g. pkg:pypi/openai), the name field — even if it's "openai" — is no longer checked. The old code would have flagged it via the name branch. This is the intended fix (trust the purl over the name), but the spec update only documents the ecosystem-scoping rule for purl matches; it doesn't state that a purl's presence suppresses the name fallback. A reader of the spec won't know why name: "openai" + purl: "pkg:pypi/openai" produces no finding. Add one sentence to the spec.

Inconsistency early return skips adjudicate_agentic_posture for all sbom_dir states

The early return is correct for the stated goal (no contradictory notice). However, adjudicate_agentic_posture also produces a PresentButUnverified notice when sbom_dir is None. If someone passes --sbom-dir alongside an internally inconsistent binding, the "SBOM present but contradicts" path is also silently skipped. For spec 210's current scope this is fine — the error is already fatal — but the behaviour gap between "sbom_dir supplied" and "sbom_dir absent" in the skipped code isn't noted anywhere.

Security

MAX_BOM_DEPTH constant visible only inside the function body

Rust's const inside a function is reachable from nested fn items in that block — this compiles correctly. No issue, but worth noting the access pattern is non-obvious.

No file-size guard before serde_json::from_slice (pre-existing, not introduced here). The depth limit added here is a good partial fix for the adversarial-BOM surface; a size cap on the bytes passed in would complete it. Out of scope for this PR but worth tracking.

Test Coverage Gaps

  1. Name-only fallback (no purl) is untested in new tests. The path None => name == Some(entry.name.as_str()) is exercised only by the old parity fixture; there's no unit test that constructs a BOM component without a purl and asserts the name match fires. The new purl_matcher_gates_on_ecosystem test only exercises the purl path.

  2. Malformed purl (missing pkg: prefix)purl_matches now returns false when split_once('/') doesn't see a pkg: prefix (ptype = ""). No test covers this. It's the correct silent-reject behavior, but a test pinning it would prevent a future refactor from accidentally making malformed purls match.

  3. MAX_BOM_DEPTH truncation is untested. A BOM nested 65 levels deep should return None; there's no test confirming the guard fires.

Spec-Spine Compliance

Both behavioral changes (ecosystem-gating on purl type and skip-on-inconsistency) are reflected in specs/001-certificate-verify-core/spec.md. The .derived/ shard hashes are regenerated. No new uses: references, so no SHA-pinning obligation. Coupling gate should pass.

Performance

No concerns. The depth guard improves worst-case behavior for adversarial input.


Automated review by Claude. Diff size: 104 lines.

@bartekus
bartekus merged commit 6452bfb into main Jul 9, 2026
9 checks passed
@bartekus
bartekus deleted the 210-verify-followups branch July 9, 2026 00:14
bartekus added a commit that referenced this pull request Jul 9, 2026
First release since v0.3.0, bundling PRs #12 and #13 (OAP spec 210 verify half).
Additive feature, so this is a minor bump under 0.x.

- Added (#12, spec 001): verify-certificate carries and adjudicates the
  agenticPostureBinding: round-trips it through the typed cert struct so the
  self-hash and signature hold, then adjudicates internal consistency, an SBOM
  cross-check of a declared none under --sbom-dir, and the spec 210 FR-004
  governance-envelope shape check.
- Added (#13, spec 001): ecosystem-scoped the agentic-SDK watchlist (npm openai
  no longer matches pkg:pypi/openai; purl match anchors on the name segment) and
  skipped the cross-check when internal consistency already failed, dropping the
  contradictory unknown-posture notice.

Version bumped in lockstep across Cargo.toml, npm/package.json (main + five
platform pins), and py/pyproject.toml; Cargo.lock and the codebase-index shard
regenerated.
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.

1 participant