ci: guard against committed credentials

[fedorov]

The repo is public and had essentially no safeguard against a service-account key being
committed. .gitignore covered .env and *.env; GitHub secret scanning and push protection were
disabled; there was no gitleaks/trufflehog anywhere. (bandit is a code linter — it does
not detect committed credentials.)

The existing history is clean. I scanned all 70 commits across every ref with gitleaks, and
separately grepped every blob for private-key headers, GCP/GitHub/AWS key patterns, and
service-account JSON. Nothing. So this is prevention, not cleanup.

Why this repo specifically

dev/deployment.md instructs operators to fetch each tier's deployer service-account JSON key
from that tier's deployment bucket. Those keys routinely sit in a working tree. Before this PR,
every name they realistically land under was trackable:

TRACKABLE  gcp-sa-key.json      TRACKABLE  application_default_credentials.json
TRACKABLE  idc-dev-deployer.json  TRACKABLE  key.pem, id_rsa, .envrc
TRACKABLE  credentials.json     TRACKABLE  .env.local, .env.prod

Note *.env matches prod.env but not .env.prod. One git add -A away from publishing a
deployer key for a GCP project.

Three layers, weakest last

1. Push protection (GitHub secret scanning) — already enabled on the repo via settings, so
   it is not in this diff. Rejects the push before the secret reaches GitHub. The only layer that
   prevents rather than detects. Does not apply to pushes on forks.
2. gitleaks on every PR (.github/workflows/gitleaks.yml) — covers fork PRs, and flags
   generic private-key blocks and service-account JSON that provider pattern-matching misses.
   Deliberately not path-filtered, unlike ci.yml: a secret can land in any file.
   Pinned and checksum-verified, mirroring actionlint.yml. fetch-depth: 0 because a secret is a
   leak even if a later commit "removes" it — the blob stays reachable.
3. .gitignore — the filename shapes above. Weakest layer, since no list is exhaustive; it
   exists to catch the common git add -A.

--redact keeps matched key material out of the Actions log, which is world-readable on this repo.

Verification

• gitleaks git --redact over this repo: no leaks found, 70 commits scanned.
• Planted a throwaway RSA key inside a service_account JSON in a scratch repo: gitleaks
  detected it and exited 1 (so the check fails), and the key material did not appear in the
  output — redaction works.
• Confirmed all 12 previously-trackable secret filenames are now ignored, that no tracked file
  becomes ignored by the new patterns, and that .env.example remains committable.
• actionlint clean on the new workflow.

If a key ever does get committed

SECURITY.md now says it plainly: rotate first. Deleting the commit does not un-leak it — the
blob stays reachable and this repo is public. Purge from history afterwards, not instead.

Repo settings enabled alongside this PR

These are server-side settings, not files, so they cannot appear in a diff. All three are now on,
and SECURITY.md records that they are meant to be on — otherwise nothing stops them being
switched off without review:

Setting	Status	Why
Secret scanning	enabled	Detects credentials already pushed
Secret scanning push protection	enabled	Rejects the push before the secret lands
Dependabot alerts + security updates	enabled	pip-audit fails a PR whose deps carry a known CVE; it does nothing about a CVE disclosed after merge. Security updates open a fix PR against main instead of waiting for someone to notice.

Dependabot security updates are the repo setting, not .github/dependabot.yml — that file
schedules weekly grouped version updates. Security fixes stay ungrouped so they ship on their own.

Verify any time:

gh api repos/ImagingDataCommons/IDC-REST-MCP \
  --jq '.security_and_analysis | {secret_scanning, secret_scanning_push_protection, dependabot_security_updates}'

Still not fixed

The durable fix for the underlying exposure is keyless auth — with Workload Identity Federation
there is no GCP_SA_KEY JSON to leak at all. That is tracked as future work.

---

Drafted with Claude Code.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds defense-in-depth measures to prevent committed credentials in a public repository, combining CI secret scanning, expanded .gitignore coverage for common credential filenames, and updated security guidance for incident response.

Changes:

• Add a new GitHub Actions workflow that runs gitleaks on every PR (and on pushes to main) with full history and redacted findings.
• Expand .gitignore to ignore common environment file variants and typical cloud credential / key file names.
• Update SECURITY.md to document the new credential hygiene layers and emphasize “rotate first” if a secret is ever committed.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File	Description
SECURITY.md	Documents the new credential hygiene approach and references the new gitleaks workflow.
.gitignore	Adds ignore patterns for common env/credential/key filenames to reduce accidental commits.
.github/workflows/gitleaks.yml	Introduces PR-time secret scanning with gitleaks (full history, checksum-verified install, redacted output).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[fedorov]

Thanks — half right, and the half that's right is a real doc bug. Both claims checked rather than taken on faith:

"CI is path-filtered" — correct, and now fixed. ci.yml filters pull_request on src/idc_api/**, tests/**, pyproject.toml, and uv.lock. This PR is its own proof: it touches only .gitignore, SECURITY.md, and a workflow, so it ran actionlint and gitleaks only — no pytest, no bandit, no pip-audit. PR #17, which touched src/idc_api/__init__.py, did get the test matrix. The bullet said "CI checks on every PR" and that was simply false. Reworded in 2a662d6 to name what runs unconditionally (gitleaks — deliberately unfiltered, since a credential can land in any file) versus what's gated on paths.

"no ruff format" — was true of this branch's base, not of main. ruff format --check was added to ci.yml in #15 / #16. This branch had been cut from a stale local main that predated them, so the file you reviewed genuinely lacked the step. Good catch on the symptom. I've rebased onto current main (8100fbf), and ci.yml now shows Lint (ruff), Format (ruff), Security lint (bandit), Dependency vulnerability scan (pip-audit), Test (pytest) — so "lint + format" is accurate and stays.

Net: the bullet now reads

gitleaks runs on every PR — deliberately not path-filtered […] ci.yml runs ruff (lint + format), bandit, pip-audit, and the tests suite on Python 3.11/3.12 — but only for PRs touching src/idc_api/**, tests/**, pyproject.toml, or uv.lock. […] A docs-only PR therefore runs gitleaks and nothing else.

Re-verified after the rebase: actionlint clean, gitleaks finds no leaks across history, credential filenames still ignored, and no tracked file is swallowed by the new patterns.

---

Drafted with Claude Code.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

[fedorov]

Correct — fixed in the latest commit.

ci.yml's pull_request.paths has five entries and I documented four, dropping .github/workflows/ci.yml itself. As you say, that implies a PR editing only ci.yml would skip the Python suite, when in fact it triggers it — which is exactly the case where you'd most want the suite to run, since the change is to the CI definition.

The bullet now reads "…src/idc_api/**, tests/**, pyproject.toml, uv.lock, or ci.yml itself."

I added a mechanical check rather than re-eyeballing: parse pull_request.paths out of ci.yml and assert every entry appears in the SECURITY.md bullet. It passes. Noting for the record that this sentence duplicates the workflow's filter list, so it can drift again — you've now caught two separate bugs in it. If it drifts a third time, the right move is to stop enumerating and just link to the workflow.

---

Drafted with Claude Code.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.