Skip to content

ci: add dependency and container scanning for Temps' own supply chain - #187

Merged
dviejokfs merged 7 commits into
gotempsh:mainfrom
bherila:feature/soc2-dependency-container-scanning-upstream-pr
Jul 5, 2026
Merged

dviejokfs merged 7 commits into
gotempsh:mainfrom
bherila:feature/soc2-dependency-container-scanning-upstream-pr

Conversation

@bherila

@bherila bherila commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds dependency and container-image scanning for Temps' own supply chain (as opposed to crates/temps-vulnerability-scanner, which only scans deployed customer application images — crates/temps-vulnerability-scanner/src/trivy.rs, daily_scanner.rs). Tracked as bherila#23.

  • .github/dependabot.yml (new): version-update PRs for

    • cargo (workspace root, /)
    • bun (/web — web/bun.lock confirms bun, not npm/yarn/pnpm, per CLAUDE.md; Dependabot's native bun ecosystem went GA Feb 2025 and supports the text-based bun.lock format this repo uses)
    • github-actions (workflow files, including this PR's new workflow)

    All three run weekly (Monday 06:00 America/New_York) and group minor+patch updates into a single PR per ecosystem to cut down on PR spam across 51 crates + the web deps; major bumps stay ungrouped so each gets its own PR and full CI signal, since majors are the ones most likely to need a real code change.

  • .github/workflows/dependency-scan.yml (new): runs on pull_request (path-filtered to **/Cargo.toml, **/Cargo.lock, web/package.json, web/bun.lock, and the workflow file itself), on a weekly schedule (Monday 06:00 UTC), and via workflow_dispatch.

    • cargo-audit: rustsec/audit-check@v2.0.0 against the workspace Cargo.lock. Fails the job on any RUSTSEC advisory match.
    • web-audit: bun install + bun audit --audit-level=high in web/. Fails the job on high/critical severity findings.
    • container-scan (stretch goal, implemented): Trivy scan of the currently published ghcr.io/<owner>/temps:latest image (CRITICAL,HIGH, ignore-unfixed), with best-effort SARIF upload to GitHub code scanning (continue-on-error: true since Advanced Security may not be enabled everywhere). This only runs on schedule/workflow_dispatch, not pull_request — release.yml only builds+pushes images on v*.*.* tags, so a PR has no fresh image to scan; scanning the last-published :latest weekly is the meaningful signal here.

Fail vs. warn

Chose fail for cargo-audit and bun audit (job returns non-zero on high/critical findings) rather than warn-only: this is a net-new capability, so there's no existing "required check" to break, and a scanner that only prints warnings tends to get ignored. Same threshold (CRITICAL,HIGH) is used for the Trivy job for consistency.

Heads-up: running bun audit --audit-level=high against web/'s current lockfile as of this PR already reports 29 existing vulnerabilities (1 critical, 28 high) in transitive deps (react-router, handlebars via a transitive dep, d3-color, flatted, picomatch). That means this workflow will show red on the first PR/schedule run against main, not because the workflow is broken, but because it's surfacing real pre-existing debt for the first time. Recommend a fast-follow PR to bump/patch those before treating this check as a merge gate.

Verification

  • python3 -c "import yaml,sys; yaml.safe_load(open(sys.argv[1]))" run against both new YAML files — both parse cleanly.
  • Confirmed web/bun.lock (not package-lock.json/yarn.lock/pnpm-lock.yaml) is present, confirming bun is the actual package manager.
  • Ran bun audit --audit-level=high directly against web/ locally — confirmed it installs, runs, and exits non-zero on the existing high/critical findings (see heads-up above).
  • Verified rustsec/audit-check@v2.0.0's action.yml directly (it's a bundled Node 20 action, not a from-source cargo install, so it doesn't hit slow compiles in CI) and confirmed its token input matches what's wired up here.
  • Verified aquasecurity/trivy-action@v0.36.0's action.yaml directly against the upstream repo (not from memory) — confirmed image-ref, format, output, severity, exit-code, and ignore-unfixed are all real, current inputs, and that tags use a v prefix (v0.36.0, not 0.36.0).
  • Attempted a local cargo install cargo-audit --locked to do an end-to-end run of the underlying tool against this workspace's Cargo.lock; the from-source build didn't finish in the sandbox's time/CPU budget (shared box, heavy gix/rustls dependency tree) so I killed it rather than let it run indefinitely. Since rustsec/audit-check doesn't build from source in CI (see above), this doesn't block the workflow from working — but I did not get a completed local cargo audit run against this repo's actual Cargo.lock, so flagging that explicitly rather than claiming full end-to-end verification of that one piece.

Follow-ups (not done here)

  • Fix/triage the 29 existing bun audit findings in web/ (see heads-up above) so the new check goes green.
  • Consider adding CodeQL (mentioned in the original issue) as a separate, larger PR — out of scope here, which is specifically dependency/container scanning.

Adds cargo-audit and bun audit checks (pull_request + weekly schedule)
plus a Dependabot config for cargo/bun/github-actions, and a best-effort
weekly Trivy scan of the published release image. Temps' existing
vulnerability scanner only covers deployed customer application images,
not Temps' own crates, web dependencies, or build images.

Refs #23

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MmJbBkcFSnk9ER6WA66gF2
@bherila
bherila force-pushed the feature/soc2-dependency-container-scanning-upstream-pr branch from 3ee880f to 1e68b45 Compare July 3, 2026 19:47
bherila and others added 5 commits July 3, 2026 21:50
…ched versions

- git2 0.20.4 -> 0.21.0 (RUSTSEC-2026-0184: unsound null-pointer Signature
  from Blame); updates one call site's .name() Result handling
- memmap2 transitively bumped past 0.9.10 (RUSTSEC-2026-0186: unchecked
  pointer offset)
- web: react-router-dom bumped to ^7.18.1; handlebars, d3-color, flatted,
  picomatch pinned to patched versions via package.json overrides
…commit

crates/temps-auth/src/handlers.rs was accidentally swept into the prior
"fix(deps)" commit by an unrelated in-progress edit that was present in
this shared worktree at commit time. That work (concurrent-session
detection on login, #24) is not part of this dependency
security fix; reverting the file here to keep this PR's diff scoped to
the git2/memmap2/web dependency bumps it was opened for.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Mw8dhP1gAcYRmJwT72xvM
quick-xml 0.37.5 (RUSTSEC-2026-0194/0195) is a direct temps-dns
dependency, bumped in Cargo.toml with no API changes needed.
anyhow 1.0.102 (RUSTSEC-2026-0190) moved to a patched release within
its existing ^1.0 constraint via lockfile update only.

Remaining audit findings need bigger changes and are left for a
separate pass: remove_dir_all/tempdir (transitive via the external
nixpacks crate), rsa (transitive via jsonwebtoken/octocrab), protobuf
(transitive via prometheus/pingora-core -- pingora is core proxy
infra, not something to bump lightly), and rustls-pemfile (already at
its ^2.0 ceiling; a fix needs a 3.x major bump). 8 other findings are
RustSec 'unmaintained' notices, informational only.
…only Rust advisories

Web: force minimatch/tar/lodash/defu to patched versions via package.json
overrides (all were transitive through eslint/@typescript-eslint,
tailwindcss-oxide, and openapi-ts codegen). Verified `bun install`,
`bun run lint`, and `bun run build` all still succeed with the forced
versions. Note: `bun audit --ignore` is silently non-functional in
bun 1.3.14 (tested with both GHSA slugs and bun's internal numeric
advisory ids) -- overrides is the actual fix, not a workaround.

Rust: ignore RUSTSEC-2024-0370 (proc-macro-error2), RUSTSEC-2025-0134
(rustls-pemfile), and RUSTSEC-2018-0017 (tempdir) via cargo-audit's
built-in ignore list -- these are "unmaintained crate" notices, not
exploitable CVEs, and all three are transitive through pingora/nixpacks
with no upstream fix available yet.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Mw8dhP1gAcYRmJwT72xvM
…audit

Fixes the failing Rust Dependency Audit job (run 28694843923, job
85102598339). cargo-audit was flagging 3 real vulnerabilities that
have no available fix and are already fully documented as
unreachable/blocked-upstream in Cargo.toml's 'Dependency Overrides -
Security Fixes' section, but the workflow's ignore list was never
updated to match that analysis:

- RUSTSEC-2024-0437 (protobuf 2.28.0 via prometheus <- pingora):
  vulnerable recursive-parse path is compiled in but never fed
  attacker input (pingora only emits metrics, never parses protobuf).
- RUSTSEC-2023-0018 (remove_dir_all 0.5.3 via tempdir <- nixpacks):
  TOCTOU race lives in a nixpacks code path temps-presets never
  exercises (always passes an explicit out_dir).
- RUSTSEC-2023-0071 (rsa 0.9.10 via jsonwebtoken <- octocrab/
  openidconnect, the Marvin Attack): no fixed rsa release exists
  upstream; our usage is RS256 JWT sign/verify, not the private-key
  decryption path the timing side-channel targets.

Verified locally: 'cargo audit' with the updated --ignore list exits
0 against this branch's Cargo.lock.
@dviejokfs

Copy link
Copy Markdown
Contributor

@bherila Picked up the failing Rust Dependency Audit (cargo-audit) check (run 28694843923, job 85102598339) and pushed a fix directly to this branch (23759b5a).

Root cause: cargo audit was flagging 3 real (non-"unmaintained") vulnerabilities that this branch's own Cargo.toml already had fully researched and documented as unreachable/blocked-upstream (see the "Dependency Overrides - Security Fixes" section, items 1, 2, and 5) — but the workflow's ignore: list was never updated to match that write-up, so the job kept failing on findings that had already been triaged:

  • RUSTSEC-2024-0437 — protobuf 2.28.0 via prometheus 0.13 ← pingora-core 0.8.x. The vulnerable recursive-parse path is compiled in, but pingora only ever emits metrics via TextEncoder, never parses protobuf-encoded input — not reachable. Blocked upstream: both pingora-core 0.8.0 and the latest 0.8.1 still pin prometheus ^0.13, and a [patch.crates-io] to protobuf 3.x doesn't compile against prometheus 0.13's generated code.
  • RUSTSEC-2023-0018 — remove_dir_all 0.5.3 via tempdir ← nixpacks 1.41.0 (still crates.io latest). The TOCTOU race only fires in nixpacks' implicit-output-dir branch; temps-presets always passes an explicit out_dir, so that branch is dead code here.
  • RUSTSEC-2023-0071 (Marvin Attack) — rsa 0.9.10 via jsonwebtoken ← octocrab (GitHub App JWT) / openidconnect (OIDC). No fixed rsa release exists upstream at all yet. Our usage is RS256 JWT sign/verify, not the private-key decryption path the timing side-channel targets, and the signing key isn't exposed to a remote timing oracle.

I re-verified all three reachability claims still hold against main as of today before ignoring them, added the three RUSTSEC IDs to the cargo-audit step's ignore: list (mirroring the existing "unmaintained crate" entries), and cross-referenced the workflow comment back to the Cargo.toml section so the two don't drift again. Confirmed locally: cargo audit with the updated ignore list now exits 0 against this branch's Cargo.lock.

Didn't touch anything else on the branch (web-audit / container-scan jobs untouched) — just the cargo-audit job that was linked.

…ath)

A security-auditor review of my prior commit (23759b5) found the
existing rsa/Marvin Attack (RUSTSEC-2023-0071) risk write-up was
incomplete and imprecise in two ways:

1. Missing dependency path: crates/temps-dns/src/providers/gcp.rs
   uses the rsa crate DIRECTLY (RsaPrivateKey + pkcs1v15::SigningKey)
   to sign GCP Cloud DNS service-account JWTs. This wasn't mentioned
   anywhere -- the write-up only covered the jsonwebtoken<-octocrab/
   openidconnect path.

2. Imprecise characterization: the Marvin Attack is specifically a
   chosen-ciphertext attack against RSA *decryption*; the prior text
   blurred signing and decryption together. Temps has zero RSA
   decryption usage, only signing (GitHub App JWTs + GCP DNS JWTs),
   which is a narrower, much-harder-to-exploit timing side-channel.

Updated both Cargo.toml and the CI comment to list all three
dependency paths, correctly scope what Marvin actually targets, and
note the per-customer (not shared-secret) blast radius plus a
follow-up to migrate to ring/aws-lc-rs-backed signing.

The --ignore decision itself (RUSTSEC-2023-0071 ignored in CI) is
unchanged and still verified locally: cargo audit exits 0.
@dviejokfs

dviejokfs commented Jul 4, 2026 •

Copy link
Copy Markdown
Contributor

Correction to my previous comment — after posting the cargo-audit fix, I ran a security-auditor pass specifically on the RUSTSEC-2023-0071 (rsa/Marvin Attack) risk-acceptance, since that's a real unfixed CVE and I wanted an independent check before letting it stand. It found my initial write-up (copied from this branch's existing Cargo.toml comment) was incomplete in two ways, now fixed in da3b5e94 [latest commit]:

  1. Missing dependency path. crates/temps-dns/src/providers/gcp.rs uses the rsa crate directly (RsaPrivateKey + pkcs1v15::SigningKey) to sign GCP Cloud DNS service-account JWTs. Neither the original Cargo.toml comment nor my first pass mentioned this — only the jsonwebtoken <- octocrab/openidconnect path was documented. It's a real gap in the risk record.

  2. Imprecise framing. The Marvin Attack is specifically a chosen-ciphertext attack against RSA decryption (a Bleichenbacher-style padding oracle). My first comment blurred signing and decryption together. Temps has zero RSA decryption usage anywhere — only signing, on 4 call sites total (3 GitHub App JWT paths in temps-git, 1 GCP DNS JWT path in temps-dns). That's a real but much narrower and harder-to-exploit timing side-channel than what the CVE title suggests.

The bottom line is unchanged — the security-auditor's verdict is "approve with follow-up": ignoring RUSTSEC-2023-0071 in CI is still justified, because (a) no RSA decryption exists to serve as a chosen-ciphertext oracle, (b) the signing operation's timing is buried inside 100-500ms of network I/O to GitHub/GCP that a remote attacker can't isolate to nanosecond precision, (c) the signed payload isn't attacker-controlled, and (d) each key is customer-supplied (their own GitHub App / GCP service account), not a shared temps secret, so blast radius is bounded per-customer even in a worst case. No fixed rsa release exists upstream yet regardless.

I've updated both Cargo.toml and the workflow comment to list all 3 dependency paths accurately and added a tracked follow-up: monitor upstream rsa for a constant-time release, and evaluate migrating both signing paths to ring/aws-lc-rs-backed signing instead of waiting on that.

Re-verified locally after this correction: cargo audit with the ignore list still exits 0. Sorry for the churn — wanted to flag this rather than leave inaccurate security reasoning sitting in the repo.

@dviejokfs

Copy link
Copy Markdown
Contributor

could you review the PR after the fixes? @bherila

@dviejokfs
dviejokfs merged commit e4bc14d into gotempsh:main Jul 5, 2026
15 checks passed
@bherila

bherila commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

fixes LGTM, thank you @dviejokfs

@bherila
bherila deleted the feature/soc2-dependency-container-scanning-upstream-pr branch July 5, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants