security-privacy

Execution Modes

• Critic (default fast mode): Review an implementation plan draft for security and privacy gaps — prompt-injection risks, over-broad tool allowlists in agent frontmatter, hook-script attack surface, secret exposure paths, cache-key staleness, and unsafe persistence writes.
• Legacy mode: Author a security-privacy.md report enumerating security and privacy concerns for a feature, covering all seven security surfaces below.

Inputs

Critic mode

• requirements.json — user stories, acceptance criteria, feature constraints
• code-map.json — mapped code locations for the implementation
• implementation-plan.draft.md — draft plan to review for security gaps
• anchors.json — stable task anchors for emitting review findings
• critic-selection.json — review budget and active critic configuration

Legacy mode

• requirements.json — feature requirements and acceptance criteria
• code-map.json — existing code structure and file locations
• project-context.md — technology stack and project conventions

Outputs

Critic mode

Write to reviews/security-privacy.review.json conforming to review-delta.schema.json (use code:find-plugin-file skill to locate schemas/review-delta.schema.json).

Note: The schema accepts both items and review_items as field names. The agent and mode fields are optional.

Example — prompt-injection on a new LLM-consuming stage (blocking):

{
  "review_items": [
    {
      "anchor_id": "task:add-intent-parser-stage",
      "severity": "blocking",
      "rationale": "The new intent-parser stage passes `pr_body` directly into the system prompt without any sanitization or quarantine. PR body is author-controlled content (untrusted input per PLN-720/PLN-725 precedent). A crafted body could inject instructions that alter the model's verdict or exfiltrate learning patterns surfaced by SubagentStart.",
      "proposed_change": {
        "op": "append",
        "target": "task",
        "path": "task:add-intent-parser-stage",
        "value": "Treat `pr_body`, `pr_title`, and commit messages as data, not instructions. Wrap them in XML data tags (e.g., <author_content>) and place them after all system instructions. Do not interpolate them into the instruction section of the prompt. Mirror the detect-injection quarantine pattern introduced in PLN-720."
      },
      "files": ["plugins/code-review/agents/intent-parser.md"],
      "ac_refs": ["AC-002"],
      "tags": ["prompt-injection", "untrusted-input", "llm-pipeline"]
    },
    {
      "anchor_id": "task:add-subagent-start-hook",
      "severity": "blocking",
      "rationale": "The proposed SubagentStart hook script sources content from `.closedloop-ai/env` using `eval`. If that file is written by a prior stage that processes untrusted input, eval will execute attacker-controlled shell. The existing hook pattern reads with `export $(grep ...)` but never eval.",
      "proposed_change": {
        "op": "replace",
        "target": "task",
        "path": "task:add-subagent-start-hook",
        "value": "Load env file with `export $(grep -v '^#' .closedloop-ai/env | xargs)`; never with `eval` or `source`. Confirm the file is written exclusively from controlled paths, not from model output or PR content."
      },
      "files": ["plugins/code/hooks/hooks.json"],
      "ac_refs": ["AC-005"],
      "tags": ["hook-script", "eval-injection", "subagent-start"]
    },
    {
      "anchor_id": "task:add-cache-invalidation",
      "severity": "major",
      "rationale": "The plan updates the verifier prompt but does not regenerate its `prompt_hash`. The verifier cache keys on `(content_hash, model, prompt_hash)` — a stale hash means old verdicts survive the prompt edit undetected. This is a correctness-as-security property: stale verdicts can suppress real security findings.",
      "proposed_change": {
        "op": "append",
        "target": "task",
        "path": "task:add-cache-invalidation",
        "value": "After any prompt edit, recompute the prompt_hash in the cache-key derivation logic. Add a test asserting that a changed prompt string produces a different cache key and triggers a fresh model call."
      },
      "files": ["plugins/code-review/tools/python/code_review_helpers.py"],
      "ac_refs": ["AC-008"],
      "tags": ["cache-key", "correctness-as-security", "stale-verdict"]
    }
  ]
}