From 17992801c2b80b07af1ab4446365266619208329 Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Thu, 9 Jul 2026 17:55:00 -0400 Subject: [PATCH 1/4] ci: guard against committed credentials The repo is public and nothing stopped a service-account key from being committed. History is clean (verified with gitleaks over all 70 commits), so this is prevention, not cleanup. Three layers, weakest last: - GitHub secret-scanning push protection, enabled on the repo (settings, not in this diff). Rejects the push before the secret reaches GitHub -- the only guard that prevents rather than detects. - gitleaks on every PR, not path-filtered: a secret can land in any file, and fork PRs are not covered by push protection. Pinned + checksum- verified, matching actionlint.yml. --redact keeps key material out of the Actions log, which is public here. - .gitignore for the names deployer keys land under. dev/deployment.md tells operators to fetch per-tier SA JSON keys, so `gcp-sa-key.json` and friends routinely sit in a working tree one `git add -A` from a commit. Previously only `.env` and `*.env` were covered -- not `.env.local`, and no JSON/PEM shape at all. SECURITY.md documents the layers and the response: rotate first, purge second. Deleting the commit does not un-leak a pushed key. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/gitleaks.yml | 65 ++++++++++++++++++++++++++++++++++ .gitignore | 27 +++++++++++++- SECURITY.md | 19 ++++++++-- 3 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/gitleaks.yml diff --git a/.github/workflows/gitleaks.yml b/.github/workflows/gitleaks.yml new file mode 100644 index 0000000..6e8520f --- /dev/null +++ b/.github/workflows/gitleaks.yml @@ -0,0 +1,65 @@ +name: Secret scan (gitleaks) + +# Scans the git history for committed credentials. This is defense-in-depth behind GitHub's +# secret-scanning push protection, which rejects a push *before* the secret reaches GitHub: +# - push protection does not apply to pushes on a fork, so a fork PR can carry a leaked key +# into review; this job runs on those PRs. +# - push protection only knows provider patterns it recognizes; gitleaks also flags generic +# private-key blocks and service-account JSON, which is the shape of this project's +# deployer credentials (see dev/deployment.md). +# +# Deliberately NOT path-filtered — unlike ci.yml, which only runs when source changes. A secret +# can be committed in any file, so every PR is scanned. +# +# `--redact` keeps matched secret material out of the Actions log, which is public on this repo. + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: gitleaks-${{ github.ref }} + cancel-in-progress: true + +env: + GITLEAKS_VERSION: "8.30.1" + # sha256 of gitleaks__linux_x64.tar.gz from the release's checksums.txt + GITLEAKS_SHA256: "551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb" + +jobs: + gitleaks: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + # Full history: a secret is a leak even if it was "removed" in a later commit, since + # the blob stays reachable. Shallow clones would scan only the tip. + fetch-depth: 0 + + - name: Install gitleaks (pinned + checksum-verified) + run: | + set -euo pipefail + TARBALL="gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" + curl -fsSLO "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/${TARBALL}" + echo "${GITLEAKS_SHA256} ${TARBALL}" | sha256sum -c - + tar -xzf "$TARBALL" gitleaks + ./gitleaks version + + - name: Scan history + run: | + set -euo pipefail + # Exits non-zero when a leak is found, failing the check. + ./gitleaks git --redact --no-banner . + + - name: What to do if this failed + if: failure() + run: | + echo "::error::A credential was found in the git history." + echo "Rotate the credential FIRST — assume it is compromised the moment it is pushed." + echo "Removing the commit does not un-leak it: the blob stays reachable, and this repo is public." + echo "Then purge it from history (git filter-repo) and force-push. See SECURITY.md." diff --git a/.gitignore b/.gitignore index e270f0e..f106eee 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,31 @@ dist/ .vscode/ .DS_Store -# Secrets +# Secrets — first line of defense only. GitHub push protection (rejects the push) and the +# gitleaks CI job (fails the PR) are the backstops, since no filename list is exhaustive. .env +.env.* *.env +!.env.example +.envrc + +# Cloud credentials. Deployer service-account keys are fetched per tier (see dev/deployment.md), +# so they routinely land in a working tree — these are the names they land under. +*-key.json +*_key.json +*-keys.json +*-deployer.json +gcp-*.json +service-account*.json +service_account*.json +credentials.json +client_secret*.json +application_default_credentials.json + +# Private keys / certificates +*.pem +*.key +*.p12 +*.pfx +id_rsa +id_ed25519 diff --git a/SECURITY.md b/SECURITY.md index 2291f7f..78a479c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -36,8 +36,23 @@ exhaustion) rather than data disclosure. Full rationale and the guarded-SQL thre themselves, not sensitive data — the cap is about log-line hygiene (one pathological query can't inflate a line), not confidentiality. Client IPs are not logged at the application level (Cloud Run's own request log already has caller IP, correlatable by timestamp). -- **CI checks** on every PR: `ruff` (lint), `bandit` (static security lint), `pip-audit` - (dependency CVEs), and the `tests` suite. +- **CI checks** on every PR: `ruff` (lint + format), `bandit` (static security lint), `pip-audit` + (dependency CVEs), `gitleaks` (committed credentials), and the `tests` suite. +- **Credential hygiene, in three layers.** The service itself holds no secrets, but the *deploy* + path does: each tier's deployer service-account JSON key. Those live in GitHub **Environment + secrets**, never in the repo — and three independent guards keep them out of it: + 1. **Push protection** (GitHub secret scanning) rejects a push containing a recognized + credential *before* it reaches GitHub. This is the only guard that prevents rather than + detects. It does not cover pushes to forks. + 2. **[gitleaks](.github/workflows/gitleaks.yml)** scans the full history on every PR, including + from forks, and flags generic private-key blocks and service-account JSON that provider + pattern-matching misses. Findings are redacted in the log (this repo's Actions logs are + public). + 3. **`.gitignore`** covers the filenames deployer keys actually land under. It is the weakest + layer — a filename list is never exhaustive — and exists to catch the common `git add -A`. + + If a credential is ever committed: **rotate it first.** Deleting the commit does not un-leak it; + the blob stays reachable and this repo is public. Purge from history afterwards, not instead. - **Non-root container** — `Dockerfile` drops to an unprivileged user before serving. ## Known residual risks (public deployment) From 8daf3e776f795244506bbad905ca948b932f6c17 Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Thu, 9 Jul 2026 18:01:37 -0400 Subject: [PATCH 2/4] docs(security): record the repo-level security settings Enabled on the repository (Settings -> Code security), alongside the secret-scanning work in this PR: - Dependabot alerts + automated security updates. pip-audit only fails a PR whose deps already carry a CVE; a CVE disclosed after merge went unnoticed until someone looked. Security updates are the repo setting, not dependabot.yml -- that file schedules weekly *version* updates. These are server-side settings, invisible in any diff, so they can be switched off without review. SECURITY.md is now the record that they are meant to be on, with the gh command to verify. Co-Authored-By: Claude Opus 4.8 --- SECURITY.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/SECURITY.md b/SECURITY.md index 78a479c..5c5c623 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -38,6 +38,12 @@ exhaustion) rather than data disclosure. Full rationale and the guarded-SQL thre (Cloud Run's own request log already has caller IP, correlatable by timestamp). - **CI checks** on every PR: `ruff` (lint + format), `bandit` (static security lint), `pip-audit` (dependency CVEs), `gitleaks` (committed credentials), and the `tests` suite. +- **Dependency vulnerabilities, caught twice.** `pip-audit` fails CI on a PR whose dependencies + carry a known CVE, and **Dependabot alerts + automated security updates** are enabled on the + repository, so a CVE disclosed *after* a PR merges still opens a fix PR against `main` rather + than waiting for someone to notice. [dependabot.yml](.github/dependabot.yml) separately schedules + weekly grouped *version* updates for the `uv` and `github-actions` ecosystems; security updates + are the repo setting, not that file, and are ungrouped so a fix ships on its own. - **Credential hygiene, in three layers.** The service itself holds no secrets, but the *deploy* path does: each tier's deployer service-account JSON key. Those live in GitHub **Environment secrets**, never in the repo — and three independent guards keep them out of it: @@ -53,6 +59,16 @@ exhaustion) rather than data disclosure. Full rationale and the guarded-SQL thre If a credential is ever committed: **rotate it first.** Deleting the commit does not un-leak it; the blob stays reachable and this repo is public. Purge from history afterwards, not instead. + +> **These are repository settings, not files.** Secret scanning, push protection, Dependabot alerts, +> and Dependabot security updates are all enabled under *Settings → Code security*. They are not +> visible in any diff, so they can be switched off without a code review — this section is the only +> record that they are meant to be on. Verify with: +> +> ```bash +> gh api repos/ImagingDataCommons/IDC-REST-MCP \ +> --jq '.security_and_analysis | {secret_scanning, secret_scanning_push_protection, dependabot_security_updates}' +> ``` - **Non-root container** — `Dockerfile` drops to an unprivileged user before serving. ## Known residual risks (public deployment) From 2a662d699ad687e7212f531113bed43fe5f93416 Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Thu, 9 Jul 2026 18:06:14 -0400 Subject: [PATCH 3/4] docs(security): scope the CI-checks claim to what actually runs Copilot review, PR #18: the bullet claimed "CI checks on every PR", but ci.yml is path-filtered on pull_request (src/idc_api/**, tests/**, pyproject.toml, uv.lock). A docs-only PR runs neither the tests nor bandit/pip-audit -- this PR is itself the proof: it triggered only actionlint and gitleaks. gitleaks is deliberately unfiltered, so it is the one check that really does run on every PR. Say so, and stop implying the rest do. Co-Authored-By: Claude Opus 4.8 --- SECURITY.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 5c5c623..06ae13a 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -36,8 +36,12 @@ exhaustion) rather than data disclosure. Full rationale and the guarded-SQL thre themselves, not sensitive data — the cap is about log-line hygiene (one pathological query can't inflate a line), not confidentiality. Client IPs are not logged at the application level (Cloud Run's own request log already has caller IP, correlatable by timestamp). -- **CI checks** on every PR: `ruff` (lint + format), `bandit` (static security lint), `pip-audit` - (dependency CVEs), `gitleaks` (committed credentials), and the `tests` suite. +- **CI checks.** [gitleaks](.github/workflows/gitleaks.yml) (committed credentials) runs on **every** + PR — deliberately not path-filtered, since a credential can be committed in any file. + [ci.yml](.github/workflows/ci.yml) runs `ruff` (lint + format), `bandit` (static security lint), + `pip-audit` (dependency CVEs), and the `tests` suite on Python 3.11/3.12 — but **only** for PRs + touching `src/idc_api/**`, `tests/**`, `pyproject.toml`, or `uv.lock`. `actionlint` likewise runs + only when a workflow changes. A docs-only PR therefore runs `gitleaks` and nothing else. - **Dependency vulnerabilities, caught twice.** `pip-audit` fails CI on a PR whose dependencies carry a known CVE, and **Dependabot alerts + automated security updates** are enabled on the repository, so a CVE disclosed *after* a PR merges still opens a fix PR against `main` rather From 797f692d899b82fd23e346c2a46ec7dc9fc9ced5 Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Thu, 9 Jul 2026 18:16:42 -0400 Subject: [PATCH 4/4] docs(security): include ci.yml in the documented CI path filter Copilot review, PR #18: ci.yml's pull_request.paths also lists .github/workflows/ci.yml, so a PR editing only that file does run the Python suite. The bullet named four of the five paths, implying it would not. Co-Authored-By: Claude Opus 4.8 --- SECURITY.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 06ae13a..49136b4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -40,8 +40,9 @@ exhaustion) rather than data disclosure. Full rationale and the guarded-SQL thre PR — deliberately not path-filtered, since a credential can be committed in any file. [ci.yml](.github/workflows/ci.yml) runs `ruff` (lint + format), `bandit` (static security lint), `pip-audit` (dependency CVEs), and the `tests` suite on Python 3.11/3.12 — but **only** for PRs - touching `src/idc_api/**`, `tests/**`, `pyproject.toml`, or `uv.lock`. `actionlint` likewise runs - only when a workflow changes. A docs-only PR therefore runs `gitleaks` and nothing else. + touching `src/idc_api/**`, `tests/**`, `pyproject.toml`, `uv.lock`, or `ci.yml` itself. + `actionlint` likewise runs only when a workflow changes. A docs-only PR therefore runs `gitleaks` + and nothing else. - **Dependency vulnerabilities, caught twice.** `pip-audit` fails CI on a PR whose dependencies carry a known CVE, and **Dependabot alerts + automated security updates** are enabled on the repository, so a CVE disclosed *after* a PR merges still opens a fix PR against `main` rather