Skip to content

security(ci): pin third-party GitHub Actions to full commit SHAs#358

Merged
cmeans-claude-dev[bot] merged 3 commits into
mainfrom
security/sha-pin-third-party-actions
Apr 22, 2026
Merged

security(ci): pin third-party GitHub Actions to full commit SHAs#358
cmeans-claude-dev[bot] merged 3 commits into
mainfrom
security/sha-pin-third-party-actions

Conversation

@cmeans-claude-dev
Copy link
Copy Markdown
Contributor

@cmeans-claude-dev cmeans-claude-dev Bot commented Apr 22, 2026

Closes medium-severity gap #5 from the 2026-04-21 contribution-safety audit (audit-contribution-safety-2026-04-21). Third of three planned Phase 2 PRs; docs was #357 (merged), ruleset+UI changes handled out-of-band (see decision-ruleset-main-protection-2026-04-21 in awareness).

Summary

Moving major-version tags on third-party GitHub Actions (@v6, @v3, etc.) are an unreviewed supply-chain channel — the upstream repo owner can move the tag to any commit at any time, including after a compromise. Pinning to a full 40-char commit SHA freezes the code that actually runs in CI to what was reviewed at this commit.

Each pinned line carries a trailing # <action> pinned to full commit SHA — <version> comment so:

  • The human-readable version stays visible to reviewers.
  • Dependabot's github-actions ecosystem (configured in .github/dependabot.yml) continues to open update PRs — it respects SHA pins and updates both the SHA and the version comment in lockstep.

Scope

4 files changed, +25, -7 (verified via git diff --shortstat origin/main).

File ± Notes
.github/workflows/ci.yml +2, -1 codecov/codecov-action
.github/workflows/docker-smoke.yml +4, -2 docker/setup-buildx-action, docker/build-push-action
.github/workflows/docker-publish.yml +8, -4 docker/setup-buildx-action, docker/login-action (×2), docker/build-push-action
CHANGELOG.md +11, -0 ### Security entry under [Unreleased]

7 uses: lines touched, 6 distinct action@tag pairs (docker/login-action@v4 is invoked twice in docker-publish.yml, once for GHCR and once for Docker Hub; both pinned to the same SHA).

The six pinned pairs

Each SHA was resolved from the upstream tag via gh api repos/<action>/git/refs/tags/<tag> at PR creation time, cross-verified that the exact commit also carries a specific-version tag (vX.Y.Z).

Action Old New SHA Version
codecov/codecov-action @v6 57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 v6.0.0
docker/setup-buildx-action @v3 8d2750c68a42422c14e847fe6c8ac0403b4cbd6f v3.12.0
docker/build-push-action @v6 10e90e3645eae34f1e60eeb005ba3a3d33f178e8 v6.19.2
docker/setup-buildx-action @v4 4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd v4.0.0
docker/login-action @v4 4907a6ddec9925e35a0a9e82d7399ccc52663121 v4.1.0
docker/build-push-action @v7 bcafcacb16a39f128d818304e6c9c0c18556b85f v7.1.0

What's unchanged

  • actions/checkout and actions/setup-python — first-party GitHub actions; different trust boundary (GitHub itself is the upstream). Tag-pinned @v6 as before.
  • Action versions are unchanged — the SHAs resolve to the exact same commits @v3/@v4/@v6/@v7 currently point to. No behavior change expected.
  • Inconsistency in docker-publish.yml vs docker-smoke.yml (buildx v3 vs v4; build-push v6 vs v7) is preserved. Bundling version alignment with SHA-pinning would mix concerns; Dependabot can propose alignment as its own PR.

Risks and unknowns

  • Dependabot update noise. Once this lands, Dependabot may open six update PRs (one per action family) for the next patch/minor release. That's the expected, correct behavior — the github-actions ecosystem is configured with "group all" policy in dependabot.yml, so noise is bounded.
  • Upstream repo-renames break SHA refs eventually. If e.g. docker/build-push-action is ever renamed or deleted, @<sha> won't resolve. Low probability; detection is a CI failure, not silent breakage.
  • codecov/codecov-action@57e3a136… resolves to v6.0.0, not a later v6.x. That's what the floating @v6 tag currently points to, so no behavior change from main — we just froze the current state. If codecov publishes v6.1.0 later, Dependabot will propose the update.

References

  • Audit report: audit-contribution-safety-2026-04-21 (id 0c79b026)
  • Originating handoff: awareness intention 598956c6 (claude.ai, 2026-04-19, state: active)
  • Earlier Phase 2 PR: #357 (docs template + CONTRIBUTING + README, merged)
  • Ruleset changes (not a PR): decision-ruleset-main-protection-2026-04-21 (id dedf7748)

QA

Prerequisites

None. Workflow-file change; CI executes it automatically.

Automated checks

  • docker-smoke — load-bearing. Exercises the two SHA-pinned actions in docker-smoke.yml (setup-buildx@8d2750c6…, build-push@10e90e36…) against the current Dockerfile. Green smoke = the pinned SHAs resolve and behave identically to the @v3/@v6 tags they replaced.
  • test (3.10–3.14), lint, typecheck, codecov/patchcodecov/patch is the first end-to-end exercise of codecov/codecov-action@57e3a136… in this PR's CI run. If codecov upload behaves differently than before, that's the signal.
  • docker-publish — NOT triggered by this PR (trigger is push: tags). The pinned SHAs in docker-publish.yml will get their real exercise on the next release tag push.
  • CodeQL (actions) — re-scans the three workflow files. SHA-pins shouldn't introduce new taint-flow sites; clean scan expected.
  • license/cla, QA Gate — unchanged.

Manual tests

    • Diff matches the scope claim. git diff --stat origin/main..security/sha-pin-third-party-actions shows exactly the four files above at +25, -7. Nothing else.
    • Every new uses: line is a full-SHA pin (40 hex chars). Spot-check:
      grep -E "uses: .*@[0-9a-f]{40}" .github/workflows/{ci,docker-smoke,docker-publish}.yml
      
      Expected: 7 matches, all with the trailing # <action> pinned to full commit SHA — <version> comment one line above.
    • Each SHA matches a specific-version tag on the upstream action. Optional spot-check for any one action:
      gh api repos/docker/build-push-action/tags?per_page=100 \
        --jq '[.[] | select(.commit.sha == "bcafcacb16a39f128d818304e6c9c0c18556b85f")] | map(.name)'
      
      Expected: ["v7.1.0", "v7"] — the SHA carries both the specific-version tag and the floating major-version tag.
    • First-party actions/* untouched. grep -E "uses: actions/" .github/workflows/ci.yml still shows @v6 for checkout and setup-python — no scope creep into first-party.
    • docker-smoke CI job is green on this PR. Verify in the Actions tab. This is the load-bearing integration test for the buildx + build-push pins.

Acceptance

  • ✅ CI green (all automated checks)
  • docker-smoke green — load-bearing
  • ✅ 7 uses: lines SHA-pinned across 6 distinct action@tag pairs
  • ✅ Each pin carries a human-readable version comment
  • ✅ No behavior change — SHAs resolve to same commits as the tags they replaced
  • ✅ First-party actions/* unchanged
  • ✅ Dependabot continues to own update proposals (github-actions ecosystem)
  • ✅ Single-concern; no source / test / compose changes

🤖 Generated with Claude Code

Moving major-version tags on third-party actions are an unreviewed
supply-chain channel — the upstream owner can move the tag to any
commit at any time. Pinning to full commit SHA freezes the code under
review.

Six distinct action@tag pairs pinned across three workflow files, seven
uses: lines touched (docker/login-action is invoked twice in
docker-publish.yml):

- codecov/codecov-action@v6 -> @57e3a136... (v6.0.0) in ci.yml
- docker/setup-buildx-action@v3 -> @8d2750c6... (v3.12.0) in docker-smoke.yml
- docker/build-push-action@v6 -> @10e90e36... (v6.19.2) in docker-smoke.yml
- docker/setup-buildx-action@v4 -> @4d04d5d9... (v4.0.0) in docker-publish.yml
- docker/login-action@v4 -> @4907a6dd... (v4.1.0) in docker-publish.yml (×2)
- docker/build-push-action@v7 -> @bcafcacb... (v7.1.0) in docker-publish.yml

Each pinned line carries a trailing `# <action> pinned to full commit
SHA — <version>` comment so the human-readable version stays visible
and Dependabot can still track updates via the github-actions
ecosystem. First-party actions/* (checkout, setup-python) stay
tag-pinned — different trust boundary.

Closes medium-severity gap #5 from the 2026-04-21 contribution-safety
audit (audit-contribution-safety-2026-04-21).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cmeans-claude-dev cmeans-claude-dev Bot requested a review from cmeans as a code owner April 22, 2026 11:31
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 22, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 22, 2026
@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 22, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review — PR #358

Supply-chain hardening: SHA-pinning of third-party GitHub Actions. For a security PR where the load-bearing claim is "each SHA resolves to the claimed upstream version," I verified every SHA independently against the upstream repo.

Verification performed

Step Result
Scope git diff --stat origin/main..origin/security/sha-pin-third-party-actions → 4 files, +25, -7 (ci.yml +2/-1, docker-publish.yml +8/-4, docker-smoke.yml +4/-2, CHANGELOG.md +11/-0). Matches PR body exactly.
uses: SHA-pin format grep -E "uses: .*@[0-9a-f]{40}" across the 3 workflow files → 7 matches (1 + 4 + 2). Every pin is a 40-hex-char SHA.
Other workflows (pr-labels*.yml, qa-gate.yml) ✅ No uses: lines — pure shell workflows, nothing third-party to pin. No scope creep.
Comment format ✅ Every SHA-pinned uses: line has # <action> pinned to full commit SHA — <version> on the line directly above. Consistent.
First-party actions/* unchanged actions/checkout@v6 and actions/setup-python@v6 still tag-pinned throughout — no scope creep into first-party.
CHANGELOG entry ### Security under [Unreleased]; lists all 6 action@tag pairs with SHAs, versions, and file locations; notes login-action is "two invocations" (accurate); explains the comment convention and the first-party exclusion rationale; links the originating audit.
CI rollup docker-smoke (load-bearing, exercises the two docker-smoke.yml pins end-to-end), test (3.10–3.14), lint, typecheck, codecov/patch (first exercise of the new codecov-action pin), CodeQL, license/cla — all green.

SHA verification against upstream

Each SHA verified via gh api repos/<action>/tags?per_page=100 — confirms the commit carries both the claimed specific-version tag and the floating major-version tag (so "no behavior change" holds since the @vN tag currently points at the same commit).

Action PR claim Tags at SHA (upstream) Match?
codecov/codecov-action @57e3a136… v6.0.0 ["v6.0.0", "v6"]
docker/setup-buildx-action @4d04d5d9… v4.0.0 ["v4.0.0", "v4"]
docker/login-action @4907a6dd… v4.1.0 ["v4.1.0", "v4"]
docker/build-push-action @bcafcacb… v7.1.0 ["v7.1.0", "v7"]
docker/setup-buildx-action @8d2750c6… v3.12.0 ["v3.12.0", "v3"]
docker/build-push-action @10e90e36… v6.19.2 ["v6.19.2", "v6"]

All six SHAs are real commit SHAs (not tag-object SHAs — a common trap when resolving annotated tags via git/refs/tags/…) and carry the exact version claimed. Confirmed @v6 for codecov currently points at v6.0.0 only (no v6.1.0 released), so the "floating tag and specific-version pin reference the same code right now" claim is true.

Findings

None.

What's good

  • Exactly 4 files, exactly the claimed line counts — no scope drift in any direction.
  • Every SHA independently verified against upstream — no accidental pinning to an unrelated commit or tag object.
  • Version comments make pins human-readable without losing the security benefit; Dependabot's github-actions ecosystem remains the update channel (per .github/dependabot.yml group policy).
  • First-party actions/* correctly left tag-pinned — different trust boundary, clean scoping decision.
  • Inconsistency in docker-publish.yml vs docker-smoke.yml (buildx v3 vs v4, build-push v6 vs v7) is preserved — bundling version alignment with SHA-pinning would mix concerns. Right call.
  • docker-smoke CI passed, giving end-to-end validation that the pinned buildx + build-push SHAs in docker-smoke.yml behave identically to the tags they replaced.
  • docker-publish.yml pins won't get CI exercise until the next tag push — accurately flagged in the PR body as a known "exercised at release time, not in this PR's CI" limitation.

Verdict

Ready for QA Signoff — clean supply-chain hardening with every claim independently verified. Awaiting maintainer QA Approved.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 22, 2026

QA audit — transitioning label to Ready for QA Signoff.

Supply-chain hardening PR — verified in current session:

Scope — 4 files, +25/-7 exactly. ci.yml +2/-1, docker-publish.yml +8/-4, docker-smoke.yml +4/-2, CHANGELOG.md +11/-0. No scope creep (checked pr-labels*.yml, qa-gate.yml — no third-party uses: lines, nothing to pin).

7 uses: lines, 6 distinct action@tag pairs, every SHA verified against upstream via gh api repos/<action>/tags:

  • codecov/codecov-action@57e3a136…["v6.0.0", "v6"] ✓ (claim v6.0.0)
  • docker/setup-buildx-action@4d04d5d9…["v4.0.0", "v4"] ✓ (claim v4.0.0)
  • docker/setup-buildx-action@8d2750c6…["v3.12.0", "v3"] ✓ (claim v3.12.0)
  • docker/login-action@4907a6dd…["v4.1.0", "v4"] ✓ (claim v4.1.0, used twice in docker-publish.yml)
  • docker/build-push-action@bcafcacb…["v7.1.0", "v7"] ✓ (claim v7.1.0)
  • docker/build-push-action@10e90e36…["v6.19.2", "v6"] ✓ (claim v6.19.2)

All are real commit SHAs (not tag-object SHAs) and carry the claimed specific-version tag plus the floating major-version tag — confirming "no behavior change" relative to the @vN tags they replaced. Every pin has the # <action> pinned to full commit SHA — <version> comment one line above, consistent format.

First-party actions/* (checkout, setup-python) correctly left tag-pinned — different trust boundary, no scope creep.

CI all green including docker-smoke (load-bearing end-to-end validation of the two pinned actions in docker-smoke.yml) and codecov/patch (first exercise of the new codecov-action pin).

CHANGELOG### Security under [Unreleased]; comprehensive, accurate, audit-linked.

Zero findings. Awaiting maintainer to apply QA Approved.

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 22, 2026
cmeans
cmeans previously approved these changes Apr 22, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cmeans cmeans added QA Approved Manual QA testing completed and passed and removed Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 22, 2026
Empty commit to confirm that the bot now pushes with the correct
Bot User ID (272174644) and to refresh GitHub's 'last pusher'
attribution away from the personal account. Resolves the
require_last_push_approval block on PR #358 caused by commits
being pushed over SSH (personal key) instead of HTTPS+bot-token.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed QA Approved Manual QA testing completed and passed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 22, 2026
@cmeans cmeans added QA Approved Manual QA testing completed and passed and removed Ready for QA Dev work complete — QA can begin review labels Apr 22, 2026
Trivial word swap to produce a file-changing commit — previous empty
commit didn't count as a 'reviewable push' under GitHub's
require_last_push_approval rule, which is file-change-scoped. This
commit is pushed under the corrected bot identity (Bot User ID
272174644, not App ID 3223881), so the last-pusher attribution should
now resolve to the bot account and unblock cmeans's approval.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed QA Approved Manual QA testing completed and passed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 22, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 22, 2026
@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 22, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review — PR #358 (round 3, after last-pusher workaround)

Re-reviewing after the branch-protection merge block. New head: 6149e6a (was 99bab42 when I signed off round 2).

What's new on the branch

Three commits total, all authored by cmeans-claude-dev[bot]:

SHA Subject Files Notes
99bab42 security(ci): pin third-party GitHub Actions to full commit SHAs 4 The original substantive change (round 2's QA-approved state).
2934623 ci: refresh last-pusher to bot identity 0 (empty) Empty commit under the corrected bot User ID. Did not satisfy require_last_push_approval because that rule is file-change-scoped.
6149e6a docs: tiny CHANGELOG wording nit (different → separate) 1 One-word swap to produce a file-changing commit under the bot identity so last-pusher attribution resolves to the bot account.

Re-verification

Step Result
Workflow files vs. round-2 QA-approved state git diff 99bab42..6149e6a -- .github/workflows/0 lines. The 3 workflow files are byte-identical to the state I previously verified SHA-by-SHA against upstream.
All 7 uses: SHA-pins still present ✅ Confirmed by grep -E "uses: .*@[0-9a-f]{40}" on current-head copies of ci.yml, docker-smoke.yml, docker-publish.yml — same 6 distinct SHAs as round 2.
Net diff vs. main git diff --stat origin/main..origin/security/sha-pin-third-party-actions → 4 files, +25/-7 — identical totals to the round-2 claim (the CHANGELOG word swap is +1/-1 within the same 11-line entry, so totals are unchanged).
CHANGELOG wording change "different trust boundary""separate trust boundary". Semantic equivalent; "separate" is slightly more precise for a categorical distinction, "different" was also fine. Pure style nit; no technical content change.
Commit authorship ✅ All three commits authored by the bot; no hidden maintainer commits in the branch log.
CI on current head ✅ Re-ran against 6149e6a: docker-smoke, lint, typecheck, test (3.10–3.14), codecov/patch, CodeQL, license/cla — all green.

Findings

None. The security content is unchanged from round 2's QA-approved state; the two new commits are mechanical workarounds for GitHub's require_last_push_approval branch-protection rule (previous head had maintainer SSH attribution; new head pushes are bot-attributed so maintainer approval is no longer self-approval).

Note on the empty commit

2934623 (empty) stays in history as cosmetic noise. Acceptable — the message explicitly explains its purpose, and rewriting history to drop it would just invalidate the subsequent push-identity refresh it was setting up. Not a finding.

Verdict

Ready for QA Signoff (re-applied). All round-2 verifications carry forward unchanged, plus the one new line (CHANGELOG wording) verified as a pure nit. Awaiting maintainer to re-apply QA Approved (auto-removed on push per pr-labels.yml automation).

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 22, 2026

QA audit — round 3, after the last-pusher branch-protection workaround. Re-applying Ready for QA Signoff.

New head 6149e6a adds two commits over the round-2 QA-approved head (99bab42):

  • 2934623 — empty commit under corrected bot identity (did not satisfy file-change-scoped rule)
  • 6149e6a — 1-word CHANGELOG tweak ("different" → "separate") under bot identity so last-pusher resolves to the bot account

Workflow files are byte-identical to round 2's verified state (git diff 99bab42..6149e6a -- .github/workflows/ → 0 lines). All 6 distinct SHAs still present, independently verified against upstream in round 2 — no need to re-verify.

Net diff vs. main unchanged: 4 files, +25/-7. CHANGELOG word swap is +1/-1 within the same 11-line entry.

CI re-ran on 6149e6a and is fully green (docker-smoke, lint, typecheck, test 3.10–3.14, codecov, CodeQL, license/cla).

Zero new findings. Maintainer to re-apply QA Approved (auto-removed on push by pr-labels.yml).

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge QA Approved Manual QA testing completed and passed and removed QA Active QA is actively reviewing; Dev should not push changes Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 22, 2026
@cmeans-claude-dev cmeans-claude-dev Bot merged commit d27cc6f into main Apr 22, 2026
38 checks passed
@cmeans-claude-dev cmeans-claude-dev Bot deleted the security/sha-pin-third-party-actions branch April 22, 2026 14:32
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 22, 2026
…erify) (#373)

Closes R3 of #359 (RLS harness coverage extension — tracking). Closes
#360.

## Summary

Walks the Alembic migration path (`N-1 → head`) with two-tenant data
seeded at N-1 and asserts tenant isolation still holds after applying
the head migration. Catches the class of bugs where a future migration
regresses an RLS policy with no test signal.

**Bundled fix:** R3 is the first programmatic alembic consumer in the
test suite, and in writing it we surfaced an unrelated defect in
`alembic/env.py` — `fileConfig()`'s default of
`disable_existing_loggers=True` was silencing
`mcp_awareness.postgres_store` for the rest of every pytest session that
ran migration-safety tests. One-line fix
(`disable_existing_loggers=False`) is bundled here because the test *is*
what uncovered it and the PR cannot land cleanly without it.

## Scope

**3 files changed, +235, -3** (verified via `git diff --shortstat
origin/main`).

| File | ± | Purpose |
|---|---|---|
| `tests/test_rls_migration_safety.py` | +227, -0 (new file) | The R3
test itself |
| `alembic/env.py` | +6, -3 | One-line fix + three-line comment.
`fileConfig(..., disable_existing_loggers=False)` so alembic no longer
wipes out host-app loggers when invoked programmatically. (One unrelated
quote-style byte from `ruff format` hitches a ride; verified with `git
diff`.) |
| `CHANGELOG.md` | +2, -0 | New `### Fixed` bullet for the env.py
change; existing R3 `### Security` bullet. |

## What the test asserts

Single parameterized invariant:
- alice's entry, inserted at N-1, is visible to alice and invisible to
bob both before AND after the head migration applies.

If a future head migration removes or weakens the `owner_isolation`
policy on `entries`, rewrites `app.current_user`, adds a new
owner-scoped table without RLS enabled, or otherwise regresses the
multi-tenant boundary, the post-migration assertion fails here in CI
rather than leaking data in production.

## Infrastructure

- Per-test dedicated database (`CREATE DATABASE` / `DROP DATABASE`
inside the shared `pg_container`) for full isolation without standing up
a second container. Overhead: ~0.5s per test.
- Helpers (`RLS_TEST_ROLE`, `_provision_rls_role`, `_set_rls_ctx`)
mirror those in `test_rls.py` by design. Factoring them into a shared
helper module is a follow-up refactor (noted in the file's docstring and
in the R1 PR body). Duplication is intentional here to keep R3
single-concern.
- Seeds via raw SQL (not PostgresStore) so the migration path is
exercised against the real DB schema rather than filtered through the
application layer. PostgresStore-level cross-tenant coverage at HEAD is
covered by `test_rls.py` (R1, merged in #372).

## Bundled fix: `alembic/env.py` — `disable_existing_loggers=False`

Python's `logging.config.fileConfig()` defaults to
`disable_existing_loggers=True`. That disables every logger not listed
in `alembic.ini` — including `mcp_awareness.postgres_store`.

In production, alembic runs as a one-shot CLI process, so the silencing
is invisible. In pytest, each R3 test calls
`alembic.command.upgrade(...)` in-process, which runs `env.py`, which
disables the application loggers *for the rest of the test session*. Any
test that later asserts on a log record emitted by
`mcp_awareness.postgres_store` (notably `test_do_cleanup_logs_errors`)
finds an empty `caplog.records` and fails.

Empirical confirmation on the rebased branch:

| Order | Result |
|---|---|
| `pytest tests/test_store.py::test_do_cleanup_logs_errors` (alone) |
PASS |
| `pytest tests/test_rls_migration_safety.py
tests/test_store.py::test_do_cleanup_logs_errors` | **FAIL** (empty
caplog) |
| `pytest tests/test_store.py::test_do_cleanup_logs_errors
tests/test_rls_migration_safety.py` | PASS |
| After `disable_existing_loggers=False` fix: both orderings | PASS |

The fix is the correct behavior for any long-lived host process that
invokes alembic programmatically (tests, admin scripts, server-side
migration hooks). It is not specific to R3; R3 is just the first
consumer that tripped it.

## Runtime cost

Single test, ~2.5s against a fresh testcontainers Postgres. Full pytest
suite passes at **989/989** (+1 from main's 988, attributable to this
PR's one new test). Ordering independence verified: full suite passes
regardless of test collection order after the env.py fix.

## Deferred (explicit out-of-scope)

A companion test specifically for the `_system`-schema carve-out
migration (`n9i0j1k2l3m4`) was prototyped during development. It would
verify that `_system`-owned schema entries transition from invisible (at
N-1, strict isolation) to visible (at head, carve-out applied) to other
owners. Hit an unresolved interaction between FORCE ROW LEVEL SECURITY,
BYPASSRLS, and the `owner_insert` policy's `WITH CHECK` under Postgres
17 when trying to seed the `_system`-owned row at N-1 — neither `SET
LOCAL ROLE rls_test_role` + `set_config('app.current_user', '_system')`,
nor `ALTER TABLE ... NO FORCE ROW LEVEL SECURITY` + raw superuser
insert, nor `SET LOCAL row_security = OFF` on the superuser connection
was sufficient. Runtime diagnostics confirmed FORCE=off, BYPASSRLS=true,
and permissive policies that *should* have allowed the insert, but
Postgres 17 rejected with `InsufficientPrivilege: new row violates
row-level security policy` regardless.

The core invariant R3 promised (cross-tenant isolation holds across any
migration) is fully covered by the one test shipping here. The
carve-out-specific test is queued as follow-up work once the Postgres 17
seed-path quirk is understood.

## References

- Parent tracking: #359
- Closes: #360
- Scope doc: `rls-harness-scope-2026-04-22` in awareness (id `1ec4e615`)
- R1 (merged): #372 — cross-tenant leak coverage for ~30 store methods
- Also merged today: #358 (action SHA-pinning), #375 (caplog flake fix,
which *also* touched `test_do_cleanup_logs_errors` but for a different
root cause — that PR fixed pytest's log-capture path; this PR fixes
alembic's logging-config default)

## QA

### Prerequisites

Docker (testcontainers spins up Postgres). Same as any other pytest run
in this repo. The test is self-contained; it creates its own database
inside the shared container.

### Automated checks

- `test (3.10–3.14)` — the new test runs on every matrix entry;
load-bearing
- `lint` / `typecheck` / `codecov/patch` — clean
- `docker-smoke` — not triggered (no Dockerfile / pyproject / uv.lock
touched)
- `CodeQL` — test-only + one-liner logging-config change; clean scan
expected

### Manual tests

1. - [x] **Run the new test directly:**
   ```
   python -m pytest tests/test_rls_migration_safety.py -v
   ```
   Expected: `1 passed in ~2.5s`. _QA: 1 passed in 1.99s._

2. - [x] **Meta-verification: the test is load-bearing.** Temporarily
apply a patch that weakens the `owner_isolation` policy (e.g. change it
to `USING (true)` in migration `g2b3c4d5e6f7`), re-run. Expected: the
post-migration assertion (`bob can see alice's entry`) fires and the
test fails. Revert the patch after verification. _QA: patched head
migration `n9i0j1k2l3m4` upgrade block to `USING (true)`, test failed as
expected with "bob can see alice's entry after the head migration — RLS
regressed". Restored; passes again._

3. - [x] **Per-test database isolation.** Verify that running the test
twice in a row leaves no residual databases:
   ```
   docker exec <pg_container> psql -U test -l | grep rls_migration_test
   ```
Expected: no matches (databases dropped on fixture teardown). _QA:
testcontainers removes the container on session exit so post-hoc
inspection isn't possible, but full suite (989 tests) ran cleanly across
multiple invocations — fixture cleanup logic using
`pg_terminate_backend` + `DROP DATABASE IF EXISTS` is correct._

4. - [x] **Ordering independence (bundled fix).** Run the two previously
order-sensitive tests together, both directions:
   ```
python -m pytest tests/test_rls_migration_safety.py
tests/test_store.py::test_do_cleanup_logs_errors
python -m pytest tests/test_store.py::test_do_cleanup_logs_errors
tests/test_rls_migration_safety.py
   ```
Expected: both orderings pass. Without the `alembic/env.py` fix the
first ordering fails; with the fix both pass. _QA: both orderings pass
(2.24s, 2.19s). Reverted fix to reproduce the failure: R3 then store
test → store test fails with `caplog.records: []`. Restored. Bundled fix
is genuinely required._

5. - [x] **Scope** — `git diff --stat origin/main` shows exactly:
   ```
   CHANGELOG.md                       |   2 +
   alembic/env.py                     |   9 +-
tests/test_rls_migration_safety.py | 227
+++++++++++++++++++++++++++++++++++++
   ```
   Nothing else.

### Acceptance

- ✅ CI green on all matrix entries
- ✅ 989/989 tests passing locally (988 → 989; order-independent
post-fix)
- ✅ `ruff check`, `ruff format --check`, `mypy` all clean
- ✅ Per-test database isolation (no shared state with other tests)
- ✅ Single-concern: migration-safety test + its logging-config
prerequisite
- ✅ Deferred work explicitly documented (carve-out-specific test)
- ✅ Bot commit identity verified (`272174644+cmeans-claude-dev[bot]`,
author + committer)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: cmeans-claude-dev[bot] <272174644+cmeans-claude-dev[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 23, 2026
Closes #367.

## Summary

Adds gitleaks secret-scanning at two layers:

- **Local pre-commit**: `.pre-commit-config.yaml` pins `gitleaks
v8.30.1` as a pre-commit hook. Contributors install once per clone with
`pre-commit install`. The hook scans staged diffs and blocks commits
containing secrets before they ever reach a commit object.
- **CI belt-and-suspenders**: `.github/workflows/gitleaks.yml` runs
`gitleaks/gitleaks-action@ff98106e…` (v2.3.9, SHA-pinned per the #358
action-pinning convention) on every PR and `main` push — for
contributors who didn't install the hook locally, or for paths the hook
can't cover.

## Scope

**6 files changed, +83, -5** (`git diff --shortstat origin/main`).

| File | ± | Purpose |
|---|---|---|
| `.pre-commit-config.yaml` | +12, -0 (new) | Pin gitleaks v8.30.1 as
sole pre-commit hook |
| `.gitleaksignore` | +13, -0 (new) | Allowlist 2 pre-existing
placeholder findings (both scan-mode formats) |
| `.github/workflows/gitleaks.yml` | +43, -0 (new) | CI job:
gitleaks-action@v2.3.9 on PR + main push |
| `CHANGELOG.md` | +1, -0 | `### Security` bullet under `[Unreleased]` |
| `CONTRIBUTING.md` | +13, -5 | `pre-commit install` step in Development
setup; update "Do not commit secrets" to reflect automation |
| `pyproject.toml` | +1, -0 | `pre-commit>=4.0,<5` in
`[project.optional-dependencies].dev` |

Per-file additions sum to +83 (`43+13+12+1+13+1`), matching the summary
line.

## Deviation from #367

Issue #367 proposed `pre-commit run --all-files` in CI as the
belt-and-suspenders gate. In practice, the upstream gitleaks pre-commit
hook runs `gitleaks git --pre-commit --staged` — which scans staged
diffs only. CI has nothing staged, so `pre-commit run --all-files` for
the gitleaks hook is a no-op. Using `gitleaks-action` directly instead,
which scans the PR's commit range (or full history on push).

Pre-commit framework is still installed, configured, and documented —
future hooks (ruff, mypy, etc.) can be added in follow-up PRs if desired
without re-introducing the scaffolding.

## Triage of pre-existing findings

Two findings surfaced during the pre-PR full-history scan, both in
`docs/superpowers/plans/2026-04-02-zero-downtime-deployment.md` (commit
`d7d821b`):

- Lines 166 + 438 (git-scan fingerprint): `curl-auth-user` on
`admin:haproxy-stats`
- Lines 207 + 482 (working-tree fingerprint): same pattern

**Treatment**: scoped allowlist via `.gitleaksignore`. Line 183 of the
same file explicitly instructs the operator: *"Change the stats password
(`admin:haproxy-stats`) to something from KeePass."* Production HAProxy
on holodeck is not running the published placeholder (user-confirmed
2026-04-23). The `docs/superpowers/plans/` tree is planned to migrate to
an `awareness-infra` repo, at which point the allowlist entries can be
removed.

Both fingerprint formats (commit-scoped from `gitleaks git` and
path-scoped from `gitleaks detect --no-git`) are listed in
`.gitleaksignore` so either scan mode suppresses cleanly.

## Why `gitleaks-action` and not an inline `gitleaks` invocation

- Handles `fetch-depth: 0` and commit-range selection natively for PR vs
push triggers
- Honors `.gitleaksignore` at repo root without additional config
- Uploads a SARIF artifact that GitHub Advanced Security surfaces in the
Security tab
- Pinned by commit SHA (`ff98106e…`) per the established convention
(#358)

`GITLEAKS_LICENSE` is not required — the repo is under a personal
account, not an org.

## References

- Closes: #367
- Related: #358 (action SHA-pinning convention), #378 (pr-labels
workflow push — unblocked by the `workflows` permission grant made
during this PR)
- Gitleaks docs: <https://github.com/gitleaks/gitleaks>,
<https://github.com/gitleaks/gitleaks-action>

## QA

### Prerequisites

- `pip install -e ".[dev]"` — picks up the new `pre-commit>=4.0,<5` dev
dep.
- Optional for manual hook exercise: Docker (to run `gitleaks` directly
without installing the binary). The pre-commit framework pulls the
gitleaks binary into its own isolated env automatically — no extra
install needed for the hook.

### Manual tests

1. - [x] **Pre-commit framework installs cleanly.** From a fresh venv:
`pip install -e ".[dev]" && pre-commit install`. Expected: `pre-commit
installed at .git/hooks/pre-commit`. (If `core.hooksPath` is set in the
local repo, `pre-commit install` refuses — unset it first: `git config
--unset-all core.hooksPath`. Fresh clones don't have this issue.)

2. - [x] **Full-repo scan via the hook is clean.** Run `pre-commit run
--all-files` in a clean working tree. Expected: `Detect hardcoded
secrets................Passed`. (Note: because the gitleaks hook scans
staged diffs, `--all-files` is effectively a cold-start smoke test. The
real scan happens on staged diffs.)

3. - [x] **Staged diff containing a secret is blocked.** Create a dummy
file with a synthetic AWS-style access key ID and stage it, then attempt
a commit:
   \`\`\`bash
   printf 'AKIAQYLPMN5HEXAMPLEX\n' > ./scratch-secret.txt
   git add scratch-secret.txt
   git commit -m "test: should be rejected by gitleaks"
   \`\`\`
Expected: hook reports `Detect hardcoded secrets...Failed`, exits
non-zero, HEAD is unchanged, no commit is created. Clean up: \`git
restore --staged scratch-secret.txt && rm scratch-secret.txt\`.

**Note on the test secret (why this value specifically):** gitleaks v8
applies an entropy gate to the `aws-access-token` rule — the common
documentation-style `AKIAIOSFODNN7EXAMPLE` and `AKIA1234567890ABCDEF`
are too dictionary-like to clear it and will silently pass the hook.
`AKIAQYLPMN5HEXAMPLEX` (entropy 3.58) clears the gate while remaining
obviously synthetic. Please do not swap it back to a lower-entropy
literal without re-verifying the hook still fires. The gitleaks CLI
equivalent for checking a candidate string is `echo 'CANDIDATE' |
gitleaks detect --no-git --pipe --verbose` (or point the Docker image at
a file as this PR's preflight did).

**Why not `pre-commit run --from-ref HEAD --to-ref HEAD`:** that
invocation evaluates a 0-commit range (`HEAD..HEAD`) and the hook
reports `(no files to check) Skipped` regardless of what's staged — the
range is empty by construction. The staged-diff check is what
`pre-commit run` (no args) does by default when invoked against a staged
index, which is why running it via `git commit` (as above) is the
canonical exercise.

4. - [x] **Allowlisted placeholders are suppressed.** Run `pre-commit
run --all-files`. Expected: no findings on
`docs/superpowers/plans/2026-04-02-zero-downtime-deployment.md` despite
the `admin:haproxy-stats` pattern appearing four times in the file.

5. - [x] **CI gitleaks job runs on this PR.** In the PR's Checks tab, a
new `gitleaks / scan` job appears alongside the existing `test`, `lint`,
`typecheck`, `docker-smoke`, `CodeQL` jobs. It runs to completion and
passes green.

6. - [x] **SKIP env var still works for genuine emergencies.** Stage a
benign change and run `SKIP=gitleaks git commit -m "test skip"` — the
commit proceeds without running the gitleaks hook. (Clean up: `git reset
--soft HEAD~1` to undo the test commit if you made one.) This is the
documented escape hatch; `CONTRIBUTING.md` notes it should not be used
routinely.

### Acceptance

- ☐ CI green across all matrix entries including the new `gitleaks /
scan` job
- ✅ `ruff check src/ tests/`, `ruff format --check src/ tests/`, `mypy
src/mcp_awareness/` clean locally
- ✅ `pre-commit run --all-files` clean locally (confirms hook works and
`.gitleaksignore` suppresses placeholders)
- ✅ Single-concern: gitleaks + pre-commit scaffolding only (no ruff/mypy
hook additions, no unrelated docs cleanup)
- ✅ Pre-existing findings triaged: allowlist with inline justification;
user-confirmed production is not running the placeholder
- ✅ CHANGELOG `### Security` bullet under `[Unreleased]`
- ✅ CONTRIBUTING updated (Development setup + Do-not-commit-secrets)
- ✅ Bot commit identity verified (`272174644+cmeans-claude-dev[bot]`,
author + committer); push attributed to bot after granting `workflows`
permission to the app installation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: cmeans-claude-dev[bot] <272174644+cmeans-claude-dev[bot]@users.noreply.github.com>
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 23, 2026
Closes #369.

## Summary

Adds a dependency-level CVE scanner to CI: `pip-audit` runs on every PR
and `main` push against a freshly-provisioned venv with the project's
full `[dev]` dependency tree installed. Any known CVE in any installed
package fails the build.

This complements Dependabot's update-PRs (which only fire *when a fix
exists*) by forcing immediate triage the moment a vuln is disclosed —
even before an upstream patch lands.

## Scope

**3 files changed, +99, -0** (`git diff --shortstat origin/main`).

| File | ± | Purpose |
|---|---|---|
| `.github/workflows/pip-audit.yml` | +61, -0 (new) | CI job: pip-audit
2.10.0 on PR + main push |
| `CONTRIBUTING.md` | +37, -0 | New § "Security scanners" documenting
gitleaks + pip-audit + the failure-policy triage path |
| `CHANGELOG.md` | +1, -0 | `### Security` bullet under `[Unreleased]` |

Per-file additions: `61+37+1 = 99` — matches the summary line.

## Design decisions

**Inline `pip-audit` CLI over `pypa/gh-action-pip-audit@v1.1.0`.**
Matches the repo's existing "install deps then run the tool" pattern
(ruff, mypy, pytest). Keeps pip-audit's version pinned in the workflow
(`pip-audit==2.10.0`) where Dependabot can see it, avoids adding another
third-party action to SHA-pin per the #358 convention, and keeps the
workflow self-contained without depending on the action's argument
surface.

**`--skip-editable`.** The project itself (`mcp-awareness-server`) is
installed in editable mode via `pip install -e`. Without
`--skip-editable`, pip-audit can't resolve it on PyPI (unpublished) and
emits a "Dependency not found on PyPI and could not be audited" warning
that looks like a hit. `--skip-editable` cleanly excludes editable
installs from the scan.

**Bootstrap-tooling upgrade before the scan.** `pip` and `setuptools`
aren't runtime dependencies of this project — they're the venv bootstrap
machinery shipped with every Python install. When they ship with known
CVEs (the Python 3.12 ubuntu-latest image currently ships with older
versions that trip 7 CVEs), those findings aren't actionable for this
project. Upgrading them first means the scan only reports on
dependencies we actually pull in.

**Any-vuln-fails policy** (exit code 1 → CI fail). Forces triage on
every new disclosure. Remediation paths documented in `CONTRIBUTING.md`:
1. Bump the dep to fix version (preferred)
2. Pin around the vuln if fix isn't out yet
3. Accept + `--ignore-vuln CVE-ID` with justification comment

## Pre-landing preflight

Ran `pip-audit==2.10.0 --skip-editable` on a clean venv with `pip
install -e \".[dev]\"` and `pip + setuptools` upgraded:

```
No known vulnerabilities found
Name                 Skip Reason
-------------------- -------------------------------
mcp-awareness-server distribution marked as editable
exit: 0
```

No pre-existing hits. The job should land green on its first run.

## References

- Closes: #369
- Related: #358 (action SHA-pinning convention applies to the
gitleaks-action used in #380; inline CLI here avoids adding another
pinned action), #367/#380 (gitleaks hook + CI — sister security-tooling
PR that just landed), `.github/dependabot.yml` (4-ecosystem Dependabot
config; pip-audit complements it by surfacing unfixed CVEs)
- pip-audit docs: <https://github.com/pypa/pip-audit>

## QA

### Prerequisites

- Docker (for the full local suite; not strictly required for the
pip-audit step alone)
- `pip install -e \".[dev]\"` if verifying locally

### Manual tests

1. - [x] **CI `pip-audit / audit` job runs on this PR.** In the PR's
Checks tab, a new `pip-audit / audit` job appears alongside the existing
`test`, `lint`, `typecheck`, `docker-smoke`, `CodeQL`, `gitleaks`, etc.
jobs. Runs to completion and exits **pass** (no vulns on current dep
tree).

2. - [x] **Job fails when a known-vulnerable dep is injected.** On a
throwaway branch from this PR, temporarily add `\"urllib3==2.4.0\"` to
`[project.optional-dependencies].dev` in `pyproject.toml` (that version
has 5 known CVEs per the PyPA advisory database). Open a draft PR.
Expected: `pip-audit / audit` job fails with a table listing `urllib3
2.4.0 CVE-2025-50181 ...` etc., exit code 1. Revert the change.

3. - [x] **Local run matches CI.** `pip install pip-audit==2.10.0 &&
pip-audit --skip-editable` in a venv with `.[dev]` installed +
`pip`/`setuptools` upgraded. Expected: `No known vulnerabilities found`,
exit 0. (If your venv pre-dates this PR, upgrade pip/setuptools first:
`pip install --upgrade pip setuptools`.)

4. - [x] **CONTRIBUTING.md § Security scanners reads correctly** on the
rendered PR diff. The failure-policy triage path (bump → pin →
allowlist) should be coherent and the allowlist mechanism
(`--ignore-vuln CVE-ID` with justification comment) should be
discoverable.

### Acceptance

- ☐ CI green across all matrix entries including the new `pip-audit /
audit` job
- ✅ `ruff check src/ tests/`, `ruff format --check src/ tests/`, `mypy
src/mcp_awareness/` clean locally
- ✅ `pre-commit run --all-files` clean locally (gitleaks passes; no
secrets in the diff)
- ✅ Single-concern: pip-audit CI + docs only (no unrelated dep bumps, no
other tooling additions)
- ✅ No pre-existing vulns on the `.[dev]` dep tree (preflight output
above)
- ✅ Failure policy + triage path documented in CONTRIBUTING.md §
"Security scanners"
- ✅ CHANGELOG `### Security` bullet under `[Unreleased]` references #369
- ✅ Bot commit identity verified (`272174644+cmeans-claude-dev[bot]`,
author + committer); workflow file push attributed to bot

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: cmeans-claude-dev[bot] <272174644+cmeans-claude-dev[bot]@users.noreply.github.com>
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 23, 2026
Closes #370. Deferred sub-items filed as #383 (HEALTHCHECK) and #384
(main-branch drift + docker-publish scan).

## Summary

Adds two trivy steps to `.github/workflows/docker-smoke.yml`:

1. **Dockerfile misconfiguration scan** — catches root-user, missing
HEALTHCHECK, chmod sprawl, etc.
2. **Image vulnerability scan** — CVEs in the base layer + installed
system packages, run against the freshly-built `mcp-awareness:pr-smoke`
image the existing docker-smoke job already produces.

Both fail CI on `HIGH,CRITICAL`. The image scan adds `--ignore-unfixed`
so only CVEs with an upstream fix available gate the build —
unfixed-upstream CVEs surface in the run log but don't fail CI (there's
nothing to remediate until the patch lands, and failing on them would
train contributors to ignore the scanner).

Action pinned by commit SHA (`aquasecurity/trivy-action@ed142fd0…`,
v0.36.0) per the #358 convention.

## Scope

**3 files changed, +59, -1** (`git diff --shortstat origin/main`).

| File | ± | Purpose |
|---|---|---|
| `.github/workflows/docker-smoke.yml` | +30, -0 | Two new trivy steps
after the existing import smokes |
| `CONTRIBUTING.md` | +28, -1 | § "Security scanners" — trivy entry +
failure policy + triage path |
| `CHANGELOG.md` | +1, -0 | `### Security` bullet under `[Unreleased]` |

Per-file additions: `30+28+1 = 59` — matches the summary line.

## Design decisions

**Integrated into `docker-smoke.yml`, not a separate workflow.** That
workflow already builds the image on PR open; adding trivy as follow-up
steps re-uses the same build and the same trigger filter (Dockerfile /
pyproject.toml / uv.lock / .dockerignore / the workflow itself). A
separate workflow would rebuild the image and add CI minutes with no
added signal. The downside — trivy only runs on image-touching PRs — is
accepted for the PR-time scanner; main-branch drift is tracked
separately in #384.

**`--ignore-unfixed` on the image scan.** Base-image slim distros always
carry a list of unpatched CVEs because Debian security teams haven't cut
the fix yet. Without the flag, CI would fail on every PR regardless of
what changed. `--ignore-unfixed` narrows the gate to "CVEs we can
actually address by rebuilding with a newer base" — that's actionable;
unfixed is not.

**HIGH/CRITICAL threshold, not MEDIUM+.** MEDIUM-level CVE traffic is
high enough to cause alert fatigue without the severity correlating
strongly with exploitability in our deployment context. Start strict at
HIGH+, loosen if we discover a class of MEDIUM issue we care about; it's
easier to lower the bar than raise it after people have tuned out.

**Dockerfile config at HIGH,CRITICAL only.** Trivy's Dockerfile ruleset
includes lots of LOW-severity best-practice reminders (e.g., `DS-0026
Add HEALTHCHECK`). Those surface in the log but don't gate CI — they're
real improvements but not urgent enough to block merges. #383 tracks the
HEALTHCHECK-specifically fix.

## Pre-landing preflight

Ran the exact CI policy against a freshly-pulled
`python:3.13-slim`-based image:

```
$ docker run --rm -v \$(pwd):/repo aquasec/trivy:latest config \
    --severity HIGH,CRITICAL /repo/Dockerfile
Misconfigurations: 0

$ docker run --rm -v /var/run/docker.sock:/var/run/docker.sock aquasec/trivy:latest image \
    --severity HIGH,CRITICAL --ignore-unfixed mcp-awareness:trivy-test
No vulnerabilities found
exit: 0
```

Informational findings (not gating CI):
- Dockerfile config scan: 1 LOW (`DS-0026: Add HEALTHCHECK`) → tracked
in #383
- Image scan (unfixed CVEs excluded): 6 HIGH affected-but-no-fix
(ncurses, systemd, libudev/libsystemd). These sit waiting for Debian to
cut the patches; nothing for us to do in the meantime.

## References

- Closes: #370
- Deferred: #383 (HEALTHCHECK), #384 (main-branch drift + docker-publish
scan — #370 item 4)
- Related: #358 (action SHA-pinning convention), #367 + #369 (sibling
security-tooling PRs just landed as #380 + #382)
- Trivy: <https://trivy.dev>,
<https://github.com/aquasecurity/trivy-action>

## QA

### Prerequisites

- Docker (for the local-verification test below). CI doesn't need
anything extra — `docker-smoke.yml` already provisions what's needed.

### Manual tests

1. - [x] **CI `docker-smoke` job runs the two new trivy steps.** In the
PR's Checks tab, the `docker-smoke` job's step list should include
*"Trivy — Dockerfile misconfiguration scan"* and *"Trivy — image
vulnerability scan"*, both after the import smokes. Both steps run to
completion and pass (no HIGH/CRITICAL findings at the policy threshold).

2. - [x] **Dockerfile misconfiguration is flagged when a HIGH-severity
rule trips.** On a throwaway branch, temporarily edit the Dockerfile to
remove `USER awareness` (so trivy's `DS-0002: Image user should not be
'root'` fires at HIGH). Open a draft PR. Expected: `docker-smoke` fails
at the *"Trivy — Dockerfile misconfiguration scan"* step with the
DS-0002 finding in the log. Revert.

3. - [x] **Image CVE is flagged when a fixable HIGH-severity pkg is
installed.** Harder to reproduce without picking a specific package
version; the easy proxy is setting `ignore-unfixed: false` temporarily
in the workflow — the currently-unfixed ncurses/systemd findings will
surface and fail the job. Revert.

4. - [x] **Local repro matches CI exit behavior.** Build the image
locally with `--pull` (grabs today's base):
   \`\`\`bash
   docker build --pull -t mcp-awareness:trivy-local .
docker run --rm -v /var/run/docker.sock:/var/run/docker.sock
aquasec/trivy:latest image \\
     --severity HIGH,CRITICAL --ignore-unfixed mcp-awareness:trivy-local
   \`\`\`
Expected: exit code 0, `No vulnerabilities found` (or a very short list
with `FixedVersion` populated — bump the base image and re-run).

5. - [x] **CONTRIBUTING.md § Security scanners + § trivy failure
policy** render coherently on the PR diff. Triage path (bump base →
adjust Dockerfile → allowlist via `.trivyignore`) is discoverable.

### Acceptance

- ☐ CI green across all matrix entries including the new trivy steps in
`docker-smoke`
- ✅ `ruff check src/ tests/`, `ruff format --check src/ tests/`, `mypy
src/mcp_awareness/` clean locally
- ✅ `pre-commit run --all-files` clean locally
- ✅ Single-concern: trivy CI + docs only (no Dockerfile edits;
HEALTHCHECK tracked in #383)
- ✅ No fixable HIGH/CRITICAL pre-existing findings on a freshly-built
image
- ✅ Failure policy + triage path documented in CONTRIBUTING.md
- ✅ CHANGELOG `### Security` bullet under `[Unreleased]` references
#370, #383, #384
- ✅ Action pinned by commit SHA per #358
- ✅ Bot commit identity verified (`272174644+cmeans-claude-dev[bot]`,
author + committer)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: cmeans-claude-dev[bot] <272174644+cmeans-claude-dev[bot]@users.noreply.github.com>
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 24, 2026
Closes #363.

## Summary

Adds static analysis to CI with two layers:

1. **Community Semgrep packs** — `p/python` and `p/owasp-top-ten` via
the `semgrep/semgrep:1.161.0` container. Broad coverage for Python
security anti-patterns without us re-deriving the wheel.
2. **Three project-specific custom rules** under `.semgrep/`, each
targeting a regression class we've identified:
- `awareness-sql-missing-owner-id` — every SQL statement that touches
`entries`, `reads`, `actions`, or `embeddings` must include `owner_id`.
Codifies the R1–R4 RLS harness's runtime contract in static form.
- `no-credential-identifier-in-logs` — rejects `logger.*` / `ctx.*`
calls that interpolate variables named `token`, `bearer`,
`authorization`, `password`, `secret`, `api_key`, etc. Zero current
sites; preventive.
- `no-sql-string-interpolation` — bans f-strings, `.format()`, and
concatenation as arguments to `cursor.execute` / `conn.execute` /
`executemany`. Codifies the parameterized-query convention the project
already follows.

## Scope

**8 files changed, +391, -1** (`git diff --shortstat origin/main`).

| File | ± | Purpose |
|---|---|---|
| `.github/workflows/semgrep.yml` | +60, -0 (new) | CI job running packs
+ `.semgrep/` rules |
| `.semgrep/awareness-sql-owner-scope.yml` | +65, -0 (new) | Custom rule
#1 |
| `.semgrep/no-token-in-logs.yml` | +64, -0 (new) | Custom rule #2 |
| `.semgrep/no-sql-string-interpolation.yml` | +66, -0 (new) | Custom
rule #3 (includes `paths.exclude` for `alembic/versions/**` and
`benchmarks/**` — see "Round 2 fix" below) |
| `docs/security/semgrep-rules.md` | +113, -0 (new) | Per-rule
documentation + exception/suppression discipline |
| `CONTRIBUTING.md` | +12, -0 | § "Security scanners" gains Semgrep
entry + § "Semgrep failure policy" section |
| `CHANGELOG.md` | +6, -0 | `### Security` bullet under `[Unreleased]` |
| `src/mcp_awareness/oauth_proxy.py` | +5, -1 | Inline `# nosemgrep`
suppression on a false-positive `python-logger-credential-disclosure`
finding, with a diff-visible justification comment |

Per-file additions: `60+65+64+66+113+12+6+5 = 391` — matches the summary
line.

## Round 2 fix (commit `51390e3`)

Round 1 (`b80b9cf`) CI failed with 5 findings from the
`no-sql-string-interpolation` rule — all in operational scripts outside
the request path:

- `alembic/versions/d5e8a3b17f92_add_embeddings_table.py`,
`f1a2b3c4d5e6_add_owner_id_and_users.py`,
`g2b3c4d5e6f7_add_rls_policies.py`, `j5e6f7g8h9i0_force_rls.py` —
migrations emit DDL via `op.execute(f"ALTER TABLE {table} ...")`
iterating a hard-coded Python-literal table list. Controlled input, zero
user data path.
- `benchmarks/semantic_search_bench.py:241,267` — ephemeral throwaway
databases dropped via `f'DROP DATABASE IF EXISTS "{db_name}"'`.
Developer-supplied name in a local benchmark script, not user input.

Root cause of the CI miss: pre-landing scan ran on `src/` + `tests/`
only (matching how the other linters are scoped) instead of the whole
repo. Fix: added `paths.exclude` to the rule for `alembic/versions/**`
and `benchmarks/**`. Documented in the rule's header comment. A
contributor introducing user-input SQL into those directories would
bypass the rule — but that would stand out in review, and the cost of
false positives on every migration PR isn't worth that marginal
coverage.

Round 2 scan (`semgrep --config p/python --config p/owasp-top-ten
--config .semgrep/ --error --oss-only` over the **whole repo**): exit 0,
no findings.

## Rule selection rationale (what's dropped from the issue's proposal)

The issue proposed 3-4 custom rules as candidates. A pre-implementation
audit of the codebase changed the viable set:

**Kept:**
- SQL owner-scoping guard (issue's rule 1) — the audit confirmed 52/66
SQL files already conform; the 14 exceptions are `users` + `sessions`
tables with their own keys, not owner-scoped. Strongly expressible in
Semgrep's generic mode.

**Dropped:**
- *Tool handlers must `await ctx.info(...)` for audit visibility*
(issue's rule 2). Audit: **0 of 32** tool handlers in `tools.py`
currently do this. The convention is aspirational, not established. A
rule that fires 0 times today and requires retrofitting 32 handlers
before it's useful is scope creep; the retrofit is its own design
decision.
- *New `postgres_store` function with `owner_id` must be in the RLS
harness coverage list* (issue's rule 4, already flagged in the issue as
*"may defer to a lint check"*). Audit confirmed: `test_rls.py` uses a
**per-method** pattern (42 hardcoded tests), not parametrization — so
Semgrep can't express the rule at all (needs cross-file function→test
mapping). Defer to a standalone check if ever needed; out of scope for
Semgrep.

**Added (not in issue, justified by audit):**
- Credential-identifier-in-logs prevention (pattern-based, not taint
mode).
- SQL-string-interpolation ban.

Net: 3 custom rules that each catch a real regression class, zero
retrofit burden, all expressible in Semgrep. Issue's "3-4 custom rules"
target met.

## Pre-landing preflight

Ran the exact CI command locally: \`semgrep --config p/python --config
p/owasp-top-ten --config .semgrep/ --error src/ tests/\`. Result: **one
finding before suppression; clean after.** The finding was Semgrep's
`python-logger-credential-disclosure` firing on `oauth_proxy.py` line
252:

> `logger.info("OAuth proxy: discovered endpoints — authorize=%s,
token=%s, register=%s", safe_url_for_log(...), ...)`

False positive: the format string contains the word *"token"* as a label
for the OIDC `token_endpoint` URL, not a credential value; values pass
through `safe_url_for_log()` which redacts embedded credentials.
Suppressed with a `# nosemgrep` comment on the `logger.info(` line plus
a multi-line justification comment directly above. `CONTRIBUTING.md`
documents the suppression-with-justification discipline.

All three custom rules were tested locally against both positive
fixtures (synthetic bad examples — each rule fires) and negative
fixtures (realistic benign code — no rule fires).

## References

- Closes: #363
- Related: #358 (SHA-pinning — the Semgrep container is tag-pinned to
`1.161.0`, which matches the pattern for third-party images with
explicit Dependabot-visible version numbers); #367/#369/#370 (sibling
security-tooling PRs just landed as #380, #382, #385); #359 / R1–R4 RLS
harness (the SQL owner-scope rule codifies its contract statically)
- Semgrep docs: <https://semgrep.dev/docs>
- `docs/security/semgrep-rules.md` — per-rule documentation

## QA

### Prerequisites

- `pip install semgrep` if verifying locally (CI runs in the
`semgrep/semgrep:1.161.0` container, no local install needed)

### Manual tests

1. - [x] **CI `semgrep / scan` job runs on this PR.** In the PR's Checks
tab, a new `semgrep / scan` job appears alongside existing jobs. Runs to
completion and passes (no findings after the in-tree suppression).

2. - [x] **Local repro matches CI.** `semgrep --config p/python --config
p/owasp-top-ten --config .semgrep/ --error --oss-only src/ tests/`.
Expected: exit code 0, no findings.

3. - [x] **Custom rule: SQL owner-scoping — positive case fires.** Write
a throwaway `src/mcp_awareness/sql/test_bad.sql`:
   \`\`\`sql
   /* name: test_bad */
   /* mode: literal */
   SELECT id FROM entries WHERE source = %s;
   \`\`\`
Run \`semgrep --config .semgrep/ src/mcp_awareness/sql/test_bad.sql\`.
Expected: `awareness-sql-missing-owner-id` fires with the blocking
severity. Remove the file when done.

4. - [x] **Custom rule: SQL owner-scoping — negative case clean.**
Replace the SELECT with `SELECT id FROM entries WHERE owner_id = %s AND
source = %s;` and rerun. Expected: no finding.

5. - [x] **Custom rule: credential-identifier-in-logs — positive case
fires.** In a throwaway `/tmp/bad_log.py`:
   \`\`\`python
   import logging
   logger = logging.getLogger(__name__)
   def bad(token): logger.info(f"got {token}")
   \`\`\`
Run \`semgrep --config .semgrep/no-token-in-logs.yml /tmp/bad_log.py\`.
Expected: `no-credential-identifier-in-logs` fires.

6. - [x] **Custom rule: SQL-string-interpolation — positive case
fires.** In a throwaway `/tmp/bad_sql.py`:
   \`\`\`python
   def bad(cur, x): cur.execute(f"SELECT * FROM t WHERE id = '{x}'")
   \`\`\`
Run \`semgrep --config .semgrep/no-sql-string-interpolation.yml
/tmp/bad_sql.py\`. Expected: `no-sql-string-interpolation` fires.

7. - [x] **Custom rule docs render on the PR diff.** Review
`docs/security/semgrep-rules.md` — each of the three rules has a
*what/why/suppression* section; the "Running locally" section's command
line matches what CI runs.

### Acceptance

- ☐ CI green across all matrix entries including the new `semgrep /
scan` job
- ✅ `ruff check src/ tests/`, `ruff format --check src/ tests/`, `mypy
src/mcp_awareness/` clean locally
- ✅ Full `semgrep --config p/python --config p/owasp-top-ten --config
.semgrep/ --error` clean locally (one false positive suppressed with
diff-visible justification)
- ✅ Each custom rule tested positively (synthetic bad fixture fires) AND
negatively (realistic benign code doesn't fire)
- ✅ Single-concern: Semgrep CI + custom rules + docs only
- ✅ Issue's 3-4 target met with 3 rules that each catch a real
regression class; dropped rules explicitly justified
- ✅ Failure policy + suppression discipline documented in
CONTRIBUTING.md and docs/security/semgrep-rules.md
- ✅ CHANGELOG `### Security` bullet under `[Unreleased]` references #363
- ✅ Bot commit identity verified (`272174644+cmeans-claude-dev[bot]`,
author + committer)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: cmeans-claude-dev[bot] <272174644+cmeans-claude-dev[bot]@users.noreply.github.com>
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 24, 2026
…#394)

## Linked issue

Fixes # none — version-stamp release, not tracked by a feature issue.

## Summary

Version stamp release for v0.18.3 (patch, 0.18.2 → 0.18.3). Renames
`[Unreleased]` → `[0.18.3] - 2026-04-24`, adds comparison link, bumps
`pyproject.toml`. No code changes.

Scope delta since v0.18.2 (13 commits, 1 runtime change):

| Category | PRs |
|---|---|
| Runtime behavior (user-visible) | **#393** — briefing surfaces
manually-fired intentions |
| CI / security tooling (no runtime change) | #392 pip-audit scope fix,
#386 Semgrep, #385 trivy, #382 pip-audit baseline, #380 gitleaks, #358
pinned action SHAs |
| Test harness (no runtime change) | #379 R4 hypothesis-fuzz RLS, #377
R2 background-thread RLS, #373 R3 migration-safety RLS, #372 R1 extended
RLS, #375 caplog flake fix |
| Docs | #357 PR template + CONTRIBUTING expansion |

Patch bump is correct: the one runtime change (#393) is a bug fix with
additive briefing fields (`urgency`, `updated`) — no API break, no
deprecations.

## Scope

```
 CHANGELOG.md   | 5 ++++-
 pyproject.toml | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)
```

Version stamp only. Zero source code changes. All code in the release
was already tested and QA-approved in its individual feature PR.

## AI-assistance disclosure

- [ ] No AI used in producing this PR
- [x] AI assisted with code generation (e.g., Copilot, Cursor, Claude
Code)
- [x] AI assisted with review / suggestions during authoring
- [x] AI assisted with the PR body or commit messages

## Review (no QA steps — all code already QA-approved in feature PRs)

Release PRs are version stamps, not new functionality. Reviewer checks:

1. - [x] `pyproject.toml` version bumped correctly (0.18.2 → 0.18.3).
2. - [x] `CHANGELOG.md` heading renamed `[Unreleased]` → `[0.18.3] -
2026-04-24` with today's date.
3. - [x] Empty `## [Unreleased]` section preserved above `[0.18.3]` for
future work.
4. - [x] Comparison links at the bottom: `[Unreleased]` now points at
`v0.18.3...HEAD`, new `[0.18.3]` link at `v0.18.2...v0.18.3`.
5. - [x] Scope delta table in this PR body matches `git log
v0.18.2..release/v0.18.3 --oneline`.
6. - [x] No source code, test, or workflow changes in the diff (strictly
version + CHANGELOG).

## Merge + tag (maintainer, post-approval)

After the QA Approved label is applied and this PR is merged, tag the
release commit:

```
git checkout main && git pull --ff-only
git tag -a v0.18.3 -m "v0.18.3 — briefing surfaces manually-fired intentions"
git push origin v0.18.3
```

The `docker-publish.yml` workflow fires on tag push and publishes
`mcp-awareness:0.18.3` + `mcp-awareness:latest`. Holodeck prod is
venv/systemd (not Docker) — deploy via `scripts/holodeck/deploy.sh` on
the operator workstation (git pull + pip + restart + HAProxy drain).
`docker-compose.yaml` uses `:latest` so no update needed there.

## Deployer note

First `get_briefing()` call on every existing owner after deploy
surfaces the accumulated fired-handoff backlog. For the production owner
that's 20+ entries since 2026-04-14. That is the intended behavior (PR
#393 fixes handoffs that were silently lost); receiving agents clear
each by transitioning off `fired` to `active`/`completed`/`cancelled`.

## Checklist

- [x] `CHANGELOG.md` heading renamed and comparison links updated
- [x] `pyproject.toml` version bumped
- [x] `README.md` — N/A, no tool count / schema / test count changes for
a release stamp
- [x] `docs/data-dictionary.md` — N/A, no schema change
- [x] `docker-compose.yaml` uses `:latest` — no update needed
- [x] No secrets, credentials, API tokens, signing keys, or `.env`
contents included
- [x] I have read and will sign the [CLA](../CLA.md) via the
`cla-assistant` bot

Co-authored-by: cmeans-claude-dev[bot] <272174644+cmeans-claude-dev[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA Approved Manual QA testing completed and passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant