fix(security): resolve Phase 3 review findings #2 (git auth off-argv) and #5 (admin-only credential lists) - #7
Merged
Conversation
Finding #2 — generic-host git credential no longer rides in the process argv. Trivy clones with go-git, which ignores GIT_ASKPASS/.netrc/credential helpers, so generic (non-GitHub/GitLab) private repos previously embedded username:token in the clone URL passed to `trivy repo` — visible via /proc/<pid>/cmdline. Generic HTTPS hosts are now cloned into tmpfs with the system `git` binary via a transient GIT_ASKPASS helper (mode 0700; echoes the credential from the clone subprocess's own environment, never argv, never the parent env, never the script file, never persisted), the requested ref is checked out, and Trivy then scans the local checkout. The helper and checkout are shredded/removed in a finally block on success, failure, or cancellation. `git` is added to the runtime image. GitHub/GitLab keep Trivy's native GITHUB_TOKEN/GITLAB_TOKEN env path (already off-argv). Finding #5 — registry and git-credential list endpoints (full metadata) are now admin-only; operators previously had read access to host/username/provider. Two new operator endpoints (GET /registries/options, GET /git-credentials/ options) return only {id, name} for scan-launch selection, exposing no credential metadata. The New Scan page uses these option endpoints. Implements docs/reviews/phase3-finding2-resolution.md (Option 1). See docs/PLAN.md §14 for both dated deviation entries and the two spec adaptations (askpass 0700 not 0600 since git execs it; async run_command on the existing tmpfs seam instead of sync subprocess).
tyler-rich
added a commit
that referenced
this pull request
Jul 3, 2026
…sion) Revises the locked stack decision from Python 3.12 to 3.13 to resolve the Grype-flagged CPython interpreter CVEs (CVE-2026-7210, -6100, -4224, -3298, -3644, -9669, -4786, ...) whose fixes exist only in Python 3.13+; the 3.12 base image was already the latest, so moving off 3.12 is the only real fix. Chose 3.13 over 3.14 for dependency-ecosystem maturity. - Dockerfile: base image python:3.12-slim-bookworm -> python:3.13-slim-bookworm (digest-pinned) for both the venv-builder and runtime stages. - pyproject.toml: requires-python >=3.13; black/ruff target-version py313. - CI: backend job runs on Python 3.13. - Remove the ci/grype.yaml ignore for the CPython interpreter binary so it is scanned and gated like everything else (the CI dogfood scan now confirms the interpreter CVEs are actually gone on 3.13, not suppressed). - Update the locked decision in CLAUDE.md and docs/PLAN.md (§0 #7, §2), mark the prior interpreter-exclusion deviation superseded, and add a dated deviation entry with the 3.13-over-3.14 rationale. Verified on Python 3.13.12: clean install of all deps (incl. cryptography and other C-extension/Rust wheels), full 325-test suite passing, a full Alembic upgrade/downgrade/upgrade cycle, and a clean app import, with no new 3.13-specific warnings (only the pre-existing Starlette HTTP_422 deprecation).
tyler-rich
added a commit
that referenced
this pull request
Jul 9, 2026
…sion) Revises the locked stack decision from Python 3.12 to 3.13 to resolve the Grype-flagged CPython interpreter CVEs (CVE-2026-7210, -6100, -4224, -3298, -3644, -9669, -4786, ...) whose fixes exist only in Python 3.13+; the 3.12 base image was already the latest, so moving off 3.12 is the only real fix. Chose 3.13 over 3.14 for dependency-ecosystem maturity. - Dockerfile: base image python:3.12-slim-bookworm -> python:3.13-slim-bookworm (digest-pinned) for both the venv-builder and runtime stages. - pyproject.toml: requires-python >=3.13; black/ruff target-version py313. - CI: backend job runs on Python 3.13. - Remove the ci/grype.yaml ignore for the CPython interpreter binary so it is scanned and gated like everything else (the CI dogfood scan now confirms the interpreter CVEs are actually gone on 3.13, not suppressed). - Update the locked decision in CLAUDE.md and docs/PLAN.md (§0 #7, §2), mark the prior interpreter-exclusion deviation superseded, and add a dated deviation entry with the 3.13-over-3.14 rationale. Verified on Python 3.13.12: clean install of all deps (incl. cryptography and other C-extension/Rust wheels), full 325-test suite passing, a full Alembic upgrade/downgrade/upgrade cycle, and a clean app import, with no new 3.13-specific warnings (only the pre-existing Starlette HTTP_422 deprecation).
tyler-rich
added a commit
that referenced
this pull request
Jul 9, 2026
…sion) Revises the locked stack decision from Python 3.12 to 3.13 to resolve the Grype-flagged CPython interpreter CVEs (CVE-2026-7210, -6100, -4224, -3298, -3644, -9669, -4786, ...) whose fixes exist only in Python 3.13+; the 3.12 base image was already the latest, so moving off 3.12 is the only real fix. Chose 3.13 over 3.14 for dependency-ecosystem maturity. - Dockerfile: base image python:3.12-slim-bookworm -> python:3.13-slim-bookworm (digest-pinned) for both the venv-builder and runtime stages. - pyproject.toml: requires-python >=3.13; black/ruff target-version py313. - CI: backend job runs on Python 3.13. - Remove the ci/grype.yaml ignore for the CPython interpreter binary so it is scanned and gated like everything else (the CI dogfood scan now confirms the interpreter CVEs are actually gone on 3.13, not suppressed). - Update the locked decision in CLAUDE.md and docs/PLAN.md (§0 #7, §2), mark the prior interpreter-exclusion deviation superseded, and add a dated deviation entry with the 3.13-over-3.14 rationale. Verified on Python 3.13.12: clean install of all deps (incl. cryptography and other C-extension/Rust wheels), full 325-test suite passing, a full Alembic upgrade/downgrade/upgrade cycle, and a clean app import, with no new 3.13-specific warnings (only the pre-existing Starlette HTTP_422 deprecation).
tyler-rich
added a commit
that referenced
this pull request
Jul 9, 2026
…sion) Revises the locked stack decision from Python 3.12 to 3.13 to resolve the Grype-flagged CPython interpreter CVEs (CVE-2026-7210, -6100, -4224, -3298, -3644, -9669, -4786, ...) whose fixes exist only in Python 3.13+; the 3.12 base image was already the latest, so moving off 3.12 is the only real fix. Chose 3.13 over 3.14 for dependency-ecosystem maturity. - Dockerfile: base image python:3.12-slim-bookworm -> python:3.13-slim-bookworm (digest-pinned) for both the venv-builder and runtime stages. - pyproject.toml: requires-python >=3.13; black/ruff target-version py313. - CI: backend job runs on Python 3.13. - Remove the ci/grype.yaml ignore for the CPython interpreter binary so it is scanned and gated like everything else (the CI dogfood scan now confirms the interpreter CVEs are actually gone on 3.13, not suppressed). - Update the locked decision in CLAUDE.md and docs/PLAN.md (§0 #7, §2), mark the prior interpreter-exclusion deviation superseded, and add a dated deviation entry with the 3.13-over-3.14 rationale. Verified on Python 3.13.12: clean install of all deps (incl. cryptography and other C-extension/Rust wheels), full 325-test suite passing, a full Alembic upgrade/downgrade/upgrade cycle, and a clean app import, with no new 3.13-specific warnings (only the pre-existing Starlette HTTP_422 deprecation).
tyler-rich
added a commit
that referenced
this pull request
Jul 26, 2026
…s structure, correct a false CVE claim in the CHANGELOG (#102) * docs(archive): make §14 contiguous, add a newest-first index, and add the finding-ID decoder Twelve dated §14 entries — every one from 2026-07-09 onward, including all the recent work — sat underneath `## Build performance` rather than under §14, so anyone scrolling §14 to the end stopped short of them. Moved the Build performance section to the end of the file instead of re-parenting the entries: it is self-contained and cross-referenced by heading name (from CLAUDE.md and four workflows), not by position, so nothing breaks. All 104 dated entries are now under §14. Added a newest-first index at the top of §14, one anchored line per entry. The entries themselves are deliberately NOT reordered: sixteen of them refer to each other relatively ("the entry below", "superseded by the entry above"), and a sort would silently invert every one. The three ordering regimes are documented instead, and the index is sorted by date regardless of physical position, so lookup no longer depends on the scroll order. Added §15, a finding-ID index: one row per SEC/SC/APIR/CON/P1-P3/D/R/QUA/INF/ FE/API/FEAT/DOC/SCN id with a one-line description and its resolving PR. §14 cites these ids bare and never re-explains them; this is the decoder that replaces the docs/reviews/ reports. It also records the SEC-* prefix collision between the two reviews that reused it. Corrected §0 locked decision #7: it said CVE-2025-15366 and CVE-2025-15367 are both unfixable on 3.14. That is true of released 3.14.6 but not of the 3.14 line — the imaplib backport landed on the maintenance branch and closes on 3.14.7 (issue #98). Only the poplib CVE remains 3.15-only (#52). * docs(changelog): correct the CVE-2025-15366 claim under [Unreleased] The Python 3.14 entry said all four waived CPython CVEs remain unfixable until 3.15 because upstream declined the backport to 3.10-3.14, and pointed at issue #52 for all of them. Both halves are false, and this text ships verbatim as the next release's notes. Checked against ci/grype.yaml and the two 2026-07-26 §14 entries: the imaplib backport for CVE-2025-15366 merged onto the CPython 3.14 branch on 2026-07-07 — 18 days before the entry was written — so it closes on 3.14.7, not 3.15, and it was regrouped into Group A alongside CVE-2026-15308 and CVE-2026-12003, tracked in issue #98. Only CVE-2025-15367 (poplib) is genuinely 3.15-only and still tracked in #52. What was true and is kept: released 3.14.6 carries neither guard, so the upgrade cleared nothing at the pinned version. * docs: strike completed roadmap items, surface the settings-level work, add two process rules ROADMAP: - Struck "Pin GitHub Actions to commit SHAs" (done in #57 — ci.yml has 8 SHA-pinned uses:, dev-nightly 3, publish 3, rescan 2) and "Frontend test runner" (done in #78 — vitest 3.2.7, "test": "vitest run", 20 test files). - Rewrote "Row-bound secret AAD", which was false as stated: row binding is implemented (secret_store.py row_aad(), L1/SEC-7, #64) and every write is row-bound. What remains is only the bulk re-encryption of legacy column-only ciphertext so the read fallback can be dropped, so it is folded into the existing "Admin bulk secret re-encryption" item. - Extended the public-repo governance checklist with five settings-level items that existed only in §14 prose and were therefore invisible: Actions workflow permissions -> read-only, confirm GHCR package visibility is public (the original check asked for Private and its premise inverted when the repo went public), delete the unused DOCKERHUB_* secrets, set the GitHub profile display name to tyler-rich, and confirm Dependabot security alerts are on. CLAUDE.md § Git & PR conventions gains two rules learned the hard way: a stacked child PR retargeted after its parent was squash-merged needs git rebase --onto (flipping the base in the UI re-computes the merge base and balloons the diff), and on: pull_request does not fire on 'edited', so a base change never re-runs CI and the green check you are looking at is from the old base. CONTRIBUTING § Releasing gains a "Before you tag" checklist — CHANGELOG [Unreleased] reviewed (it ships verbatim), THIRD_PARTY_LICENSES verified against the versions actually pulled, Dependabot PRs triaged, requirements.lock regenerated — plus the two after-tag steps: back-merge main into dev, and re-run rescan.yml. * docs: delete docs/reviews and docs/upgrades, sweep every inbound reference The twelve review reports and the Python 3.14 handoff doc held only closed findings, and sat at the same directory level as the two live documents. They are removed rather than moved to a docs/history/ subtree: the only real argument against deleting them was that §14 cites their finding ids bare and never re-explains them, and §15 (previous commit) answers that directly. Git is the archive for the rest. docs/ now contains exactly ARCHIVE.md, ROADMAP.md, and screenshots/. The originals stay retrievable — the pre-deletion commit is 0780b07, and the §14 entry records the git show incantation. Nothing was rewritten before deletion. Swept every inbound reference the audit enumerated, plus the ones it did not: CONTRIBUTING § Project layout (both directories dropped, screenshots/ added), CONTRIBUTING § API conventions, CHANGELOG's L13/APIR-8 citation, and both dependabot.yml D3 comments now point at ARCHIVE §15. Inside §14, 59 docs/reviews/ and 7 docs/upgrades/ path prefixes were stripped so the entries name the reports as documents rather than as paths that no longer resolve, with a note at the top of §14 sending the reader to §15. Fixed the dead claude-md-compliance-review.md link (a filename that never existed). Left one docs/upgrades/ mention deliberately: the 2026-07-25 entry's record of what CONTRIBUTING's layout listing omitted is a statement about that date, marked '(as it then was)'. Verified no workflow, test, or source file referenced either directory, and that the four '§ Build performance' cross-references are by heading name and survive that section's move. Added the dated §14 entry recording all of it, including that docs/history/ was considered and rejected, and two corrections to the audit: twelve entries were misfiled under Build performance (not fourteen — the other two are that section's own sub-headings), and there are 42 remote branches with 36 prunable (not 39/33).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves two findings from the Phase 3 security review. Review-only pass identified these; this PR fixes them. No other Phase 3 behavior changes.
Finding #2 — generic-host git credential no longer visible in the process list
Trivy clones repos with
go-git, which never shells out to the systemgitbinary and therefore ignoresGIT_ASKPASS,.netrc, and credential helpers. For generic (non-GitHub/GitLab) private hosts, Trivy's only credential channel is the clone URL — so the old code embeddedusername:tokenin the URL passed as a positional argv totrivy repo, making the credential visible via/proc/<pid>/cmdlineto any process in the container's PID namespace.Per the committed decision spec (
docs/reviews/phase3-finding2-resolution.md, Option 1), generic HTTPS hosts are now cloned ourselves:gitbinary.GIT_ASKPASShelper (mode0700) that echoes it from the clone subprocess's own environment — never in argv, never in the parent process env, never in the script file, never persisted.finallyon success, failure, or cancellation.gitis added to the runtime image.GITHUB_TOKEN/GITLAB_TOKENenv path (already off-argv).Clone failures raise an operator-safe error that never echoes git's stderr (which can contain the request URL).
Finding #5 — credential metadata is now admin-only
GET /api/registriesandGET /api/git-credentials(full metadata — host, username, provider) are now admin-only; operators previously had read access. Because launching a scan still needs an operator to pick a credential by name, two new operator endpoints return only{id, name}:GET /api/registries/options(enabled registries)GET /api/git-credentials/optionsThe New Scan page's credential pickers now use these option endpoints. No host, username, provider, auth type, or secret is exposed to operators.
Two adaptations from the spec (both preserve its security mechanics)
The spec assumed Alpine/
apk+ syncsubprocess; Scrye is Debian/apt+ an async worker. Logged indocs/PLAN.md§14:0700, not0600— git execs it, so a non-executable helper fails withEACCES. Still owner-only.run_commandseam on the container's tmpfs/tmp(matchingdocker_config_env), not a blockingsubprocess.runor a bespoke mount — so it doesn't block the event loop.Tests
test_credentials.pycases assert the credential never appears in the clone argv, is delivered only via env, the askpass file is0700at clone time, the workspace is shredded on both success and scan-exception, token-only auth maps correctly, commit checkout is issued, and a clone-failure message leaks no credential.test_targets_api.pyupdated: operators get403on the full lists and200on the/optionsendpoints, which expose only{id, name}.ruff/black/.env.examplesync clean. Frontendeslint/prettier/buildclean.See
docs/PLAN.md§ Deviations for the two dated entries.