[#227] feat: assess-security capability + deterministic secret-scanning layer

[rucka]

PR Information

PR Title: [#227] feat: assess-security capability + deterministic secret-scanning CI layer
Story/Epic: #227 (epic #208 — quality/security review)
Type: Feature (skill + KB + CI config — no runtime application code)
Priority: Medium
Assignee: Gianluca Carucci
Reviewers: independent blind review (2-lens: process + adversarial nit)
Labels: feature, security

Summary

What Changed

Delivers story #227's two halves:

1. pair-capability-assess-security (skill, dataset + installed mirror) — review-mode 1-line verdict + collapsed findings feeding the risk matrix's security dimension (D22); audit-mode OWASP scan writing a report and proposing project rules via record-decision. Rules resolve through a 5-layer cascade (global KB → per-service → per-web-app → adoption → package-scoped). It never scans for secrets itself (D24).
2. Deterministic secret-scanning CI layer — secret-scanning.md guideline + gitleaks-example.toml asset (root + dataset, byte-identical, byte-parity-locked by a conformance test), provisioned by /setup-gates. Dogfooded into this repo's own .github/workflows/ci.yml via the license-free gitleaks binary (pinned 8.30.1) — not gitleaks-action@v2, which requires a GITLEAKS_LICENSE for org-owned repos and fails before scanning. Verified green in live CI (secret-scan job). Root .gitleaks.toml allowlists exactly the documented synthetic strings via precise regexes.

Supporting: D22 recorded as an ADL; quality-model Security pillar backlinks the 5-layer cascade; ADR-015 records "deterministic layer is CI, not a skill"; /pair-process-review Step 2.4 composes assess-security (CHANGES-REQUESTED on introduced vulnerabilities).

Why This Change

Security findings now surface in every review against an objective, resolved rule set, and a committed secret mechanically blocks the build with no LLM involved (R6.5) — closing the two gaps in epic #208.

Story Context

User Story: I want pair-capability-assess-security (review + audit) plus a deterministic secret-scanning layer, so security findings surface in every review with objective rules and secrets mechanically block the build.
Acceptance Criteria:

• AC1 — review composes assess-security with the 5-layer resolved rule set; 1-line verdict + collapsed findings (D22). ✅
• AC2 — committed secret → build RED mechanically, no LLM (R6.5). ✅ (dogfooded, CI-verified)
• AC3 — $mode: audit → report in .pair/working/reports/security/ + proposes project rules to record. ✅
• AC4 — introduced vulnerabilities → CHANGES-REQUESTED. ✅

Changes Made

Implementation Details

• Change 1: assess-security SKILL.md (dataset + .claude/ mirror) — review/audit modes, 5-layer cascade, D22 output, D24 boundary.
• Change 2: secret-scanning.md guideline + gitleaks-example.toml (root + dataset, byte-identical) — dual CI-template forms (Action + GITLEAKS_LICENSE for orgs; binary for no-license), fail-closed, allowlist mechanism.
• Change 3: repo ci.yml secret-scan job (gitleaks binary) + root .gitleaks.toml; /setup-gates provisions the layer; /pair-process-review Step 2.4 composes assess-security.
• Change 4: ADR-015, D22 ADL, quality-model Security-pillar backlink; conformance suites; docs-site pages; skill-count 35→36 refresh; llms.txt regen.

Files Changed (42 files — see the PR Files tab for the authoritative list)

• Added: .claude/skills/pair-capability-assess-security/SKILL.md (+ dataset mirror); .pair/knowledge/guidelines/quality-assurance/security/secret-scanning.md (+ dataset mirror); .pair/knowledge/assets/gitleaks-example.toml (+ dataset mirror); .gitleaks.toml; .pair/adoption/tech/adr/adr-015-…md; .pair/adoption/decision-log/2026-07-18-d22-…md; packages/knowledge-hub/src/conformance/{assess-security,secret-scanning}.test.ts.
• Modified: .github/workflows/ci.yml; pair-process-review + pair-capability-setup-gates + pair-next SKILLs (+ mirrors); quality-assurance/quality-model.md (+ mirror); README.md; .pair/knowledge/getting-started.md + way-of-working.md (+ mirrors, 35→36 count); .pair/llms.txt (regenerated); packages/knowledge-hub/eval/trigger-prompts.json; several apps/website/content/docs/** pages.
• Deleted / Renamed: none.

Database Changes / API Changes

• N/A — skill/KB/CI content, no runtime code or data.

Testing

Test Coverage

• Conformance Tests: assess-security.test.ts (20 — modes, D22, D24, review composition, setup-gates provisioning) + secret-scanning.test.ts (16 — dual-form template, org-license documented, fail-closed, byte-parity lock for this story's mirrored files).
• CI (dogfood): the secret-scan job runs the gitleaks binary on this repo — verified green in live CI.
• Allowlist verification: full-history scan clean (0 leaks); a 1-char-different secret still trips (allowlist is surgical, not broad).

Test Results

@pair/knowledge-hub vitest: ✅ 182 passing (assess-security 20, secret-scanning 16, + suite)
skills:conformance:        ✅ PASS (36 skills, 26 capability)
docs:staleness:            ✅ PASS
check:links / markdownlint: ✅ Clean
CI: build ✅ · preview ✅ · secret-scan ✅ (binary, no license)

Testing Strategy

• Happy Path: review composes assess-security → 1-line verdict + collapsed findings.
• Edge Cases: committed secret → RED; audit mode → report + rule proposals; introduced vuln → CHANGES-REQUESTED.
• Error Handling: fail-closed CI (set -euo pipefail, --exit-code 1, no continue-on-error).

Quality Assurance

• assess-security is output-only (audit report is the one exception, D14); adoption writes go through record-decision.
• Byte-parity lock for this story's mirrored security files; conformance asserts dataset + mirror.
• No debugging artifacts.

Deployment Information

• Environment Impact: the secret-scan job becomes a required CI check on this repo; KB/skill content ships with the dataset. Config: the binary form needs no secret; the Action form (for adopters) needs GITLEAKS_LICENSE for orgs. Rollback: revert the PR.

Breaking Changes

• None for consumers. New required CI job on this repo is additive.

Documentation

• Technical Documentation: secret-scanning.md guideline, ADR-015, D22 ADL, quality-model backlink.
• User Documentation: docs-site pages updated; skill catalog 35→36.
• Knowledge Sharing: .pair/llms.txt was regenerated and swept in ~9 previously-unindexed pre-existing files (azure-devops-, context-map-, canonical-states, ddr-template, subdomain-context-template) — harmless generator output, unrelated to this story, noted here for transparency.

Performance Impact

• N/A (skill/KB/CI content). The secret-scan binary job adds ~8s to CI.

Security Considerations

Security Review

• Deterministic, fail-closed secret gate: gitleaks binary, --exit-code 1, no continue-on-error, pinned 8.30.1 — no LLM in the path (R6.5, D24).
• Allowlist scope: .gitleaks.toml allowlists exactly the documented synthetic strings (AKIAABCDEFGHIJKLMNOP, usr_abc123def456, sess_xyz789uvw012); no un-allowlisted secret-shaped string introduced; a different secret still fails.
• assess-security covers authn/authz, injection, OWASP surface via the rule cascade — the judgment-call counterpart, deliberately separate from the deterministic gate.

Security Testing

• Security Scan: the new secret-scan job itself; live-CI green; full-history scan clean.

Accessibility

• N/A — prose/MDX + CI config; no new interactive UI.

Risk Assessment

Technical Risks

Risk	Impact	Probability	Mitigation
Adopter on an org uses the Action form without a license → job fails before scanning	Med	Low	Template documents the org-license requirement + the license-free binary alternative; this repo uses the binary
gitleaks binary fetched over network without a checksum pin (version-pinned only)	Low	Low	Official GitHub release over HTTPS, version-pinned; a checksum pin is a reasonable future hardening

Reviewer Guide

Review Focus Areas

1. AC2 dogfood: the secret-scan CI job actually goes green (binary, no license).
2. D24 boundary: assess-security never scans secrets; the deterministic layer is CI, not a skill.
3. Mirror parity: security guideline + asset byte-identical root↔dataset.

Testing the Changes

git fetch origin feat/#227-assess-security-secret-scanning
git worktree add ../pair-227 feat/#227-assess-security-secret-scanning && cd ../pair-227
pnpm install && pnpm --filter @pair/content-ops build
pnpm --filter @pair/knowledge-hub test        # vitest 182 + check:links
gitleaks detect --source . --config .gitleaks.toml --no-banner --redact  # 0 leaks

Dependencies & Related Work

• Blocking Dependencies: none.
• Related: epic Unified classification & assessments → tags (risk, cost, security, impact) #208 (quality/security review); /setup-gates (provisions the layer); /pair-process-review (composes assess-security). ADR-011..016 H1 numbering + the ADR-016 rename are owned by PR chore: process/tooling cleanup — conformance-test convention, ADR naming fix, agent-execution-layer ADL amendment, KB fresh-checkout note #342 (kept out of this PR to avoid a cross-PR conflict; this PR owns only its own adr-015).
• Follow-up Work:
  • Tech Debt (pre-existing): ~30 unrelated root↔dataset mirror byte-mismatches (out of pair-capability-assess-security + deterministic secret-scanning layer #227 scope) — recommend a dedicated mirror-parity cleanup + a general parity gate; setup-gates KB refs not mirrored to root (Verify + fix: @pair/content-ops devDependency may break published pair-cli's KB install/update #345 tracks the content-ops devDependency risk).
  • Hardening (optional): add a SHA256 pin for the gitleaks binary download.

Pre-Submission Checklist

• All ACs implemented and verified (AC2 CI-verified green)
• Security guideline + asset byte-identical root↔dataset (conformance-locked)
• All checks green (vitest 182, skills:conformance 36, docs:staleness, check:links, markdownlint; CI incl. secret-scan)
• No breaking changes
• Summary + testing + security review + reviewer guide complete
• CI passing; no merge conflicts with main

[rucka]

Code Review — PR #341

PR Number: #341
Author: rucka (Gianluca Carucci)
Reviewer: Independent review (/pair-process-review, adversarial nit-hunt)
Review Date: 2026-07-18
Story/Epic: #227 (assess-security + deterministic secret-scanning layer) / Epic #208
Review Type: Feature

Review Summary

Overall Assessment

• Approved with Comments — Minor issues noted, can merge

The change is solid and the four acceptance criteria are genuinely met (not just claimed):
Step 2.4 wires assess-security as a required Phase-2 composition; setup-gates provisions a
required, fail-closed secret-scan job + .gitleaks.toml; audit mode writes to
.pair/working/reports/security/; and Step 5.2 ties an introduced red finding to
CHANGES-REQUESTED. The deterministic layer is dogfooded and live CI secret-scan passes (7s).
All cross-references resolve in both the root and dataset trees; the 4 byte-parity mirror files are
identical; quality-model.md §3.1/§3.2/§4 citations are accurate; conformance test counts are correct
(assess-security.test.ts = 20). ADR-015 numbering and the D22 ADL naming/structure conform.

This is an adversarial re-review against an author claim of zero outstanding findings. That claim
does not hold: the items below are real (one concrete defect + several nits). None are blocking.

Business Value Validation

Closes the two gaps in Epic #208 as specified: a judgment-call security assessment skill, and a
structural (LLM-free) secret gate. Confirmed.

Detailed Review Comments

Minor Issues

• packages/knowledge-hub/eval/trigger-prompts.json:166-171 (AD-N3) — Self-contradictory
  eval record. It is type: should-not-trigger with near_miss_for: "assess-*" and
  expected: "assess-security". Since assess-security is a member of the assess-* family, the
  record simultaneously asserts "no assess-* skill may fire" and "assess-security is the correct
  destination" — mutually exclusive under the schema documented in eval/README.md and the file's own
  $comment. Every other negative (AD-N1→setup-pm, AD-N2→record-decision, AD-N4→none) has expected
  pointing outside its near_miss_for. Moreover the prompt "Assess the security posture of our
  login flow" is now a genuine should-trigger for assess-security (near-identical to AS-02), so
  it is a true positive miscategorized as a near-miss. The PR description frames this as a correct fix
  ("now correctly expects assess-security"), but flipping only expected (none→assess-security) while
  leaving type/near_miss_for produced an incoherent entry. Recommendation: delete AD-N3
  (redundant with AS-01/AS-02) or convert it to a should-trigger in the assess-security family and
  drop near_miss_for. (Non-blocking: eval fixture, not asserted by vitest / not on the runtime path.)

Nits

• .pair/llms.txt — Bundles 11 added index entries unrelated to pair-capability-assess-security + deterministic secret-scanning layer #227 (Azure DevOps
  automation/issues/implementation/tracking, context-map-maintenance, context-map-template,
  canonical-states, ddr-template, subdomain-context-template) — a stale generated-index sync
  swept into a security-feature PR and not mentioned in the description. All targets exist (no dead
  links) and llms.txt is generator-produced (apps/pair-cli/src/registry/llms-generation.ts), so the
  content is correct; the only issue is scope creep + silence in the PR narrative. The one entry this
  story actually needs (secret-scanning.md) is correctly present; adoption/asset files are correctly
  excluded. Recommendation: note the regen in the description, or land it separately. Largely
  by-design (correct generator output).

• PR [#227] feat: assess-security capability + deterministic secret-scanning layer #341 description → "Changes Made / Files Changed" — Incomplete enumeration: omits
  .pair/llms.txt, .pair/knowledge/getting-started.md (+mirror), .pair/knowledge/way-of-working.md
  (+mirror), README.md, .github/workflows/ci.yml, .gitleaks.toml, and the new
  .pair/adoption/decision-log/2026-07-18-d22-…md ADL (several appear only in prose). Hedged by
  "see the PR Files tab", so low impact. Recommendation: list all changed paths or drop the partial
  list.

• .claude/skills/pair-next/SKILL.md + dataset mirror (version stays 0.4.1) — Body changed
  (35→36 catalog, 3 spots) with no version bump, whereas setup-gates (0.4.1→0.5.0) and review
  (0.5.0→0.6.0) bumped. By-design / non-actionable: no documented rule mandates a bump, and the
  35→36 count refresh was applied without version bumps everywhere else (skills-guide, 9 .mdx). pair-next
  had no behavioral change (count-only), so not bumping is the consistent choice; bumping would be the
  inconsistency. Reported for transparency.

Positive Feedback

• Genuinely test-first factual fix to secret-scanning.md's verification (--log-opts="-1" scoping),
  with the reasoning captured in ADR-015 Trade-offs.
• The secret-scanning.test.ts root↔dataset byte-parity guard (scoped to this story's 4 files) is a
  well-judged, self-documenting regression lock, honest about the ~30 pre-existing unrelated mismatches
  it deliberately does not try to boil-the-ocean fix.
• ADR-015 + the backfilled D22 ADL close two "cited-but-unrecorded decision" gaps cleanly.

Security Review

Deterministic secret-scan job is required, fail-closed (set -euo pipefail, --exit-code 1, no
continue-on-error), pinned gitleaks 8.30.1 binary (avoids the org-license gate). .gitleaks.toml
allowlists exactly 3 real synthetic strings (AKIAABCDEFGHIJKLMNOP, usr_abc123def456,
sess_xyz789uvw012) — all verified present at the cited locations; no un-allowlisted secret-shaped
string introduced. Live CI green. No security concerns.

Testing Review

assess-security.test.ts = 20 tests (matches the description), secret-scanning.test.ts = 17.
Conformance asserts both dataset and mirror copies. CI build/preview/secret-scan all pass.

Risk Assessment

Low. Findings are Minor/Nit and confined to eval fixture data + PR-hygiene; no runtime or interface risk.

---

Decision: Approved with Comments. The "zero findings" claim is disproved (AD-N3 is a real defect),
but nothing here blocks merge. Recommend fixing AD-N3 before or shortly after merge.

[rucka]

Remediation — all requested findings addressed

Fixed (this round):

• Minor — eval/trigger-prompts.json AD-N3 was self-contradictory (should-not-trigger + expected: assess-security); converted to a should-trigger case and dropped near_miss_for (the prompt is a genuine assess-security trigger).
• Nit — .pair/llms.txt regen scope now noted in the description (harmless generator output; pre-existing files indexed).
• Nit — Files Changed completed via the full PR template.
• Earlier: root README skill count 35→36; the two CI-template forms are no longer called "equivalent" (scan-scope difference stated); the secret-scanning Verification prose reconciled with the full-history dogfood CI; secret-scan CI job runs the license-free gitleaks binary — verified green in live CI.
• PR body rewritten to the full PR template (Security Review included).

Remaining — non-closable:

• pair-next version unbumped — why: count-only change; no bump rule, and bumping would be inconsistent with the other count-only refreshes (by-design).
• ~30 pre-existing root↔dataset mirror mismatches (unrelated files) — why: out of pair-capability-assess-security + deterministic secret-scanning layer #227 scope; this story's own mirrored files are byte-identical; needs a dedicated mirror-parity cleanup + gate.
• gitleaks binary no SHA256 pin — why: official GitHub release over HTTPS, version-pinned; checksum pin is an optional future hardening (noted as follow-up).

Gates green (vitest 182, skills:conformance 36, docs:staleness, check:links; CI incl. secret-scan); rebased on main.

---

Mirror reconciliation (post-merge): the root↔dataset KB mirror drift is intentionally not hand-fixed in-PR (a blind byte-sync would break the intentional dual-naming convention /X ↔ /pair-capability-X). It will be reconciled by running pair update after these PRs merge — the CLI's registry sync applies the naming transform correctly and never touches .pair/adoption/ (protected by reserved-paths). This is the intended transform-aware mechanism, not a manual sync.

[rucka]

Remediation follow-up — supply-chain hardening applied

Reconsidered the "by-design" gitleaks-download note as a real weakness and fixed it: the secret-scan job (and the shipped Option-B template, root + dataset) now pins the gitleaks binary by SHA256 (551f6fc… for 8.30.1 linux_x64, from the release checksums), verified with sha256sum -c before extraction — fail-closed if the release asset is ever swapped. Commit d0ab802.