Add threat model for the policy repo - #1794
Conversation
PR Summary by QodoAdd threat model document for conforma/policy
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
📝 WalkthroughWalkthroughAdded ChangesPolicy threat model
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Code Review by Qodo
1.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@THREAT_MODEL.md`:
- Around line 41-43: Reconcile the attestation trust boundary in
THREAT_MODEL.md: update the statements around the Rego rules and lib/intoto so
they clearly distinguish CLI/lib/intoto signature and trusted-task verification
from rule evaluation, and state the exact verification assumption for
input.attestations, including the explicit skip-verification path.
- Around line 194-196: The threat model incorrectly presents the two Quay
repositories in the push-bundles workflow as independent registry fallback.
Update the mitigation text around the GitHub Actions workflow entry to remove
that claim, or document a genuine client failover or cache path instead; do not
treat the enterprise-contract and conforma repositories as separate failure
domains.
- Around line 279-280: Update the SC-4 threat-model entry to limit go.sum to
verifying downloaded-byte integrity, not publisher provenance or intentional
malicious updates, and do not present renovate.json as a trust control. Document
exact versions for the go run dependencies github.com/conforma/cli and
oras.land/oras/cmd/oras, along with the required provenance verification or
review control for dependency updates.
- Around line 176-190: The OCI blob fetching section for ec.oci.* builtins lacks
documented controls for registry scope, redirects, credentials, response size,
and request duration. Update the “Trust boundary” documentation to identify the
exact CLI or environment controls governing each applicable behavior, and
document fail-closed handling for malformed or unavailable OCI responses; if no
control exists, explicitly state that and describe the enforced behavior.
- Line 285: Revise the DS-1 threat-model entry to remove the unsupported claim
that OPA evaluation is generally linear, unless the system’s Rego rules are
verified against OPA’s linear-time fragment and supported by benchmarks.
Document Tekton Chains’ practical attestation and SBOM size limits for
Kubernetes etcd, OCI registries, and Rekor, including each backend’s maximum
supported size, and specify whether CLI timeouts fail, drop the attestation, or
stall the pipeline.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: b5dd8b65-fdc7-4999-90e7-af79a2875bcc
📒 Files selected for processing (1)
THREAT_MODEL.md
|
🤖 Review · Started 3:29 PM UTC |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
ReviewReason: stale-head The review agent reviewed commit |
|
/fs-review |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
THREAT_MODEL.md (1)
275-275: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCorrect the trusted-task precedence documentation.
_trusted_task_rules_datacombines system and ruleData rules. Any matching effective deny rule blocks trust. A task is trusted only when no deny rule matches and at least one allow rule matches. Therefore, concatenation order and pattern specificity cannot let a ruleData allow rule override a system deny rule. Update LE-4 and remove Open Question 3.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@THREAT_MODEL.md` at line 275, Update the LE-4 entry in THREAT_MODEL.md to state that _trusted_task_rules_data combines system and ruleData rules, any matching deny blocks trust, and trust requires no deny match plus at least one allow match; clarify that concatenation order and pattern specificity cannot override a system deny with a ruleData allow. Remove Open Question 3.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@THREAT_MODEL.md`:
- Line 290: Update the DS-1 threat-model entry to distinguish OCI attestation,
manifest, and raw blob size limits, labeling the approximately 4 MiB value as
the manifest limit and removing any implication that the 500 MiB tar-entry limit
applies to ec.oci.blob raw SBOM blobs. Document the CLI evaluation timeout
behavior, including the default 5-minute --timeout and whether expiration
returns an evaluation error or may produce an undefined result.
---
Nitpick comments:
In `@THREAT_MODEL.md`:
- Line 275: Update the LE-4 entry in THREAT_MODEL.md to state that
_trusted_task_rules_data combines system and ruleData rules, any matching deny
blocks trust, and trust requires no deny match plus at least one allow match;
clarify that concatenation order and pattern specificity cannot override a
system deny with a ruleData allow. Remove Open Question 3.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 930b3567-cb65-4717-a601-380b4864ba5f
📒 Files selected for processing (1)
THREAT_MODEL.md
|
🤖 Finished Retro · ✅ Success · Started 7:32 AM UTC · Completed 7:48 AM UTC |
Retro: PR #1794 — Add threat model for the policy repoOutcome: PR merged with zero completed fullsend reviews despite four separate review dispatch attempts. All findings map to existing open issues — no new proposals needed. Timeline
What went well
Evidence for existing issues
Autonomy readinessNo autonomy-readiness assessment is possible for this PR because the fullsend review never completed. The human reviewer (joejstuart) approved without inline comments. The discarded fullsend review would have approved with 2 low findings — findings that were different from and complementary to what qodo and CodeRabbit caught. This suggests the review agent adds unique value on documentation PRs, but the pipeline reliability issues prevented that value from being delivered. |
Add THREAT_MODEL.md following the wg-agentic-sdlc 8-section schema, covering
the policy repo's specific threat surface: how Rego rules evaluate attestation
data, where logic gaps can cause silent passes, and how the policy bundle
distribution chain can be compromised.
Entry points cover attestation data, the 4-level rule_data precedence cascade,
OCI blob fetching, trusted task rules (both rules system and legacy), sigstore
configuration, volatile config exclusions, and bundle distribution. Threats are
organized into input evasion, data poisoning, logic errors, supply chain, and
denial of service categories.
Cross-references the CLI threat model (EC-2001) for CLI-level concerns rather
than duplicating them.
Notable finding: policy bundles pushed to quay.io are unsigned. Digest pinning
mitigates tag mutation but doesn't prove provenance.
Ref: https://issues.redhat.com/browse/EC-2003
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com