review-pentest

name	review-pentest
description	Weekly human-in-the-loop review of the latest automated pentest report. Pulls the newest report from pablohealth-oss GCS, summarizes findings, offers to open it, then publishes to docs/pentest/ and pablo-marketing/public/security.html via PR. Use when the user says "run /review-pentest", asks to review this week's pentest, or wants to publish the latest pentest report.

Review Pentest Skill

Weekly workflow for the owner to triage the automated /pentest output. The automated run produces a markdown artifact in gs://pablohealth-oss-compliance-reports/pentest/; this skill turns that artifact into:

1. A short triage summary the owner can react to.
2. A single published artifact: a new redacted summary row on the pablo-marketing/public/security.html reports table. The full .md report is not committed to the public repo — it stays in retention-locked GCS and goes to customers/auditors on request (see Disclosure policy). Publishing is gated on all real findings being closed.

The skill assumes the owner is already authenticated to gcloud (project pablohealth-oss) and gh.

Flow

Step 1 — pull the latest report

LATEST=$(gsutil ls gs://pablohealth-oss-compliance-reports/pentest/ \
  | grep -E '/[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}-[0-9]{2}-[0-9]{2}Z-[a-f0-9]+\.md$' \
  | sort | tail -1)
DATE=$(basename "$LATEST" | cut -c1-10)
LOCAL=/Users/kurtn/Developer/pablo/docs/pentest/${DATE}_pentest_report.md
gsutil cp "$LATEST" "$LOCAL"

This LOCAL copy is for the owner to read during triage only — do not commit it (the per-run .md no longer ships in the public repo; see Step 6). docs/pentest/*_pentest_report.md should be gitignored. If a report for $DATE is already present locally, ask the owner whether to overwrite (the runner sometimes re-uploads on retry). Default = skip if identical bytes.

Step 2 — summarize

Read the report. Produce a tight summary, structured exactly as:

• One line headline — totals + trend vs prior run (e.g. "1 HIGH, 1 MEDIUM, both carry-overs from 05-03").
• Per finding — one paragraph each, in this order: title, severity, what the report claims the root cause is, and what (if anything) is actually going on. The whole point of this review is that the LLM-generated report sometimes mis-attributes root causes (PABLO-002 on 2026-05-10 blamed AuditService for an HTTP 500 that was really a deployed-schema drift bug). Cross-check by:
  • Pulling Cloud Run logs around the run window (use the Run started and Run assembled timestamps from §1 of the report) to see the actual exception traces.
  • Diffing models.py against the latest applied alembic revision in prod (gcloud run jobs executions list --job=pablo-migrate).
  • Grepping for any code path the report says is broken — confirm before believing.
  • Dependency CVEs — always run the installed >= fixed precheck first. Before treating any pip-audit / SCA finding as real, pull the advisory from OSV (curl -s https://api.osv.dev/v1/vulns/<ID>) and compare its fixed version to what we actually run (lockfile + the deployed image's commit). If installed >= fixed, it's a false-positive from stale/just-published advisory data — not a real issue, not an accepted-risk, and no VULNERABILITY_EXCEPTIONS.md entry. This is exactly how PABLO-005 (2026-06-07) was mis-reported: pyjwt 2.13.0 flagged against advisories already fixed in 2.13.0. Confirm with a fresh local pip-audit — the runner's container DB can be staler or fresher than reality in either direction.
  • Suppressions can be silently dead. If a finding recurs week-over-week despite a prior # nosemgrep / --exclude, re-run semgrep with the exact pentest config and confirm the suppression actually fires — targeted # nosemgrep: <id> annotations break if the rule-id is truncated (semgrep needs the full <path>.<rule-name>); prefer bare # nosemgrep + a reason comment.
• Verdict — for each finding, one of:
  • real-issue (open a bead / fix). Blocks publishing the row until closed (see Disclosure policy).
  • false-positive (not a real vulnerability at all — scanner pattern hit, dependency already at/above the fix, suppression that never fired). Safe to detail fully on the public page.
  • mis-attributed (the symptom is real but the report's root-cause is wrong; explain the actual cause).
  • accepted-risk (no patch exists, no PHI reachability — needs an entry in docs/pentest/VULNERABILITY_EXCEPTIONS.md). Reserve for genuine unpatched advisories; never use it to paper over a false-positive.
  • closed (already fixed since the run; cite the PR).

Step 3 — offer to open

Ask the owner: "Open ${LOCAL} in your editor?" If yes, run open "$LOCAL". If no, continue.

Step 4 — get the owner's annotations

For every finding that ends up false-positive, mis-attributed, accepted-risk, or closed, ask the owner for the one-line public explanation that goes on the security page. Don't make these up — the marketing page is where customers see what we publish, and the wording is the owner's call. Apply the Disclosure policy below to decide how much to say. Suggested defaults the owner can accept or rewrite:

• false-positive: "Scanner pattern hit / dependency already at the fixed version — not a real vulnerability; explain why in full."
• mis-attributed: "Symptom real, root cause was X (not what the report says) — fixed in PR #N"
• accepted-risk: "No patched version exists upstream; build-time only, no PHI reachability — documented in VULNERABILITY_EXCEPTIONS.md"
• closed: "Closed in PR #N"

Disclosure policy (how much detail to publish)

The public page commits to "actionable detail held until findings are closed" and to keeping live exploit detail out of public view. Encode that as one gate plus a detail-by-category rule:

Gate — publish a run's row only once its real findings are closed. If any finding is still real-issue (open), do not publish the row yet; tell the owner it's held pending the fix bead, and offer to publish once that bead's PR merges. The only exception is an embargoed item the owner explicitly chooses to disclose early — then use the terse form below.

Detail by category (after the gate passes, every line is safe to detail):

Category	How much detail
false-positive / mis-attributed	Full detail — explain exactly why it's not a vuln. This is the strongest trust signal and carries zero attacker value; give it the most words.
closed (patch deployed)	Full detail + PR link. Describing a fixed gap no longer hands anyone a working exploit.
accepted-risk	Category + compensating control + VULNERABILITY_EXCEPTIONS.md link. No repro.
real-issue still open (embargo only)	Severity + CFR control + one-line category ONLY. Never the mechanism or repro. Normally this case shouldn't publish at all (see the gate).

Step 5 — exceptions doc (if any accepted-risk)

For each accepted-risk finding, append an entry to docs/pentest/VULNERABILITY_EXCEPTIONS.md under the ## Open section, using the format already documented in that file. Include CVE id, package + version, severity, "why not patched", compensating control, PHI impact, revisit-by date, owner.

Step 6 — publish (one PR)

First re-check the Disclosure-policy gate: if any finding is still an open real-issue, stop — the row is held until that bead's PR merges. Do not publish.

The full .md report no longer goes into the public repo. Now that Pablo handles real PHI, the full report details the live attack surface, so it goes to customers/auditors on request only — keep it in GCS (it's already there, retention-locked) and out of docs/pentest/ on the public pablo repo. Only the redacted summary row is published. (docs/pentest/ keeps the methodology + VULNERABILITY_EXCEPTIONS.md, not the per-run reports.)

So publishing is a single PR to pablo-marketing:

pablo-marketing (/Users/kurtn/Developer/pablo-marketing), branch pentest/${DATE}:

• Edit public/security.html. Insert a new <tr> at the top of the published-reports <table class="reports"> body. Current format (3 columns, no public MD link):

  <tr>
    <td>YYYY-MM-DD</td>
    <td>On request</td>
    <td>{redacted summary — totals + dynamic-controls-clean line + per-finding verdicts per the Disclosure policy; false-positives detailed fully, closed items + PR links, open real-issues NOT present because the gate holds the row}</td>
  </tr>
  

• If there were accepted-risk findings, also stage the VULNERABILITY_EXCEPTIONS.md change in the pablo repo as a separate small PR (exceptions doc is public; the report is not).
• Commit message: security: publish ${DATE} pentest summary
• Branch, push, gh pr create + gh pr merge --auto --squash.

Output to the owner

After the marketing PR merges, print:

• the merged commit SHA (marketing repo) — and the pablo exceptions-doc SHA if there was one
• the live URL: https://pablohealth.com/security#published-reports (or wherever security.html serves from)

Don't add anything else. The artifacts speak for themselves.

Rules

• Never publish a finding the owner hasn't explicitly verdict-tagged. If you can't reach the owner, stop and ask.
• Never publish a row with an open real-issue. The gate holds the whole row until that finding's bead/PR closes. Don't downgrade a real finding to ship the row sooner.
• Never publish the mechanism/repro of an open finding. If the owner explicitly chooses to disclose before the fix ships, use the terse severity + CFR + category form only.
• Detail false-positives the most. Explaining why something is not a vuln is free trust and zero attacker value — it's the point of the page.
• The full .md report never lands in the public repo now that it details a live attack surface. On request only.
• Run the installed >= fixed precheck on every dependency CVE (Step 2) before calling it real — and never write a VULNERABILITY_EXCEPTIONS.md entry for something already at/above the fix version.
• If the report claims something that contradicts what you observe in logs / source / OSV, the summary in Step 2 is where you say so. Don't paper over it.