Skip to content

feat(security): require nox/taint-analysis plugin - #95

Open
felixgeelhaar wants to merge 1 commit into
mainfrom
feat/require-taint-plugin
Open

feat(security): require nox/taint-analysis plugin#95
felixgeelhaar wants to merge 1 commit into
mainfrom
feat/require-taint-plugin

Conversation

@felixgeelhaar

Copy link
Copy Markdown
Contributor

Declares nox/taint-analysis as a required plugin so a missing taint analyzer
is visible instead of silent.

Why

The org is making nox the single owner of code-level SAST (source-to-sink
taint: injection, SSRF, path traversal) so gosec can be dropped — tracked in
klarlabs-studio/.github#14.

The shared CI installs the plugin with continue-on-error: true, deliberately:
a registry 404 that aborts the job would disable security scanning entirely
(as happened fleet-wide on 2026-07-19). The side effect was that nothing
downstream noticed when the plugin was absent — zero taint findings looked
exactly like a clean scan.

Two changes close that gap:

This PR supplies the declaration that makes the first one fire.

Change

Appends a plugins.required block to .nox.yaml. Purely additive — 9
insertions, 0 deletions; the existing scan.exclude block and its comments are
untouched, and no plugins: key existed before, so there is no duplicate
mapping key.

Validated with a YAML loader that rejects duplicate keys (yaml.safe_load
accepts them silently, which has bitten this org before), asserting that the
parsed document actually contains nox/taint-analysis under plugins.required.

Expected CI

ci / Security (nox) should stay green — the plugin installs normally, so the
new degradation gate must not fire. Reference: klarlabs-studio/axi-go#38.

Part of klarlabs-studio/.github#14.

The shared CI installs the taint plugin with continue-on-error, so a
registry outage produced zero taint findings — indistinguishable from a
clean scan. Declaring the plugin as required makes nox record the miss in
meta.degradations, which the shared go-ci gate now fails on
(klarlabs-studio/.github#57).

Part of klarlabs-studio/.github#14.

Claude-Session: https://claude.ai/code/session_01LMVPJFeTCEtUk2CSNBBBdK
@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@felixgeelhaar

Copy link
Copy Markdown
Contributor Author

Holding this PR — red CI, but not from the degradation gate

ci / Security (nox) is failing. Two separate things are going on; neither is
the new meta.degradations gate, which behaved correctly.

1. The red is pre-existing on main (not caused by this PR)

The failure is baseline fingerprint drift:

.nox/baseline.json exists but matched 0 of this scan's findings, so the gate
evaluated nothing. This is fingerprint drift, not a clean repo.

I verified this is independent of this branch by re-running the last green
main CI run (a287c3c1, no plugins.required anywhere). It now fails with
the identical error
. Cause: nox was bumped centrally 1.16.1 -> 1.23.0.

run nox baselined outcome
main 2026-07-25 (original) 1.16.1 10 green
main a287c3c1 re-run today 1.23.0 0 red — drift
this PR 1.23.0 0 red — drift

main is red right now on its own. The other three repos in this sweep
(vorhut, kraftsport-coach, lexora) also ran 1.23.0 and their baselines matched
fine (482 / 310 / 57), so this is rollops-specific re-fingerprinting, not a
fleet-wide baseline wipe.

Fix (separate PR, needs a triage decision): nox baseline migrate --prune
to re-fingerprint the 5 entries while preserving their triage reasons.

2. This PR surfaces a real, previously invisible taint finding

This is the more interesting part, and it is exactly what
klarlabs-studio/.github#14 predicted — though via a different mechanism than
expected.

Diffing the nox-results artifacts between the main re-run and this branch
(same commit content, same nox 1.23.0, only .nox.yaml differs):

PR-only findings: 1
  + TAINT-004  high  internal/pluginhost/cosign.go:32
MAIN-only findings: 0
  • main (plugin installed, not declared): taint findings = 0
  • this PR (plugin declared required): taint findings = 1

Both logs show Installed nox/taint-analysis@0.7.0 (community) and both have
meta.degradations absent. So installing the plugin was not sufficient to
make it contribute findings
— and nothing recorded a degradation. The assumed
failure mode was "registry outage -> missing plugin -> degradation recorded".
The mode actually observed here is quieter: plugin installs fine, contributes
nothing, no degradation, green check. Worth noting on
klarlabs-studio/.github#14 for the other repos that were relying on the CI
install step alone.

The finding itself — likely a false positive, but your call

TAINT-004 (CWE-22) high/high confidence
Path Traversal: keyPath flows from env_var (line 32) to os.ReadFile (line 36)
in VerifyArtifact
keyPath := os.Getenv(PublicKeyEnv)   // ROLLOPS_PLUGIN_PUBLIC_KEY
if keyPath == "" { return nil }
pubPEM, err := os.ReadFile(keyPath)

Reading a file at an operator-supplied path is the documented purpose of
ROLLOPS_PLUGIN_PUBLIC_KEY — it names the cosign public key. The source is a
process env var, not remote input, and it crosses no privilege boundary:
anyone who can set it can already read files as that process. I read it as a
false positive that wants an OpenVEX waiver with that reason rather than a code
change — but I have not baselined it, since (a) that is a triage call for you
and (b) the baseline needs migrate --prune first anyway.

Suggested order

  1. nox baseline migrate --prune + triage TAINT-004 → separate PR, turns
    main green again.
  2. Rebase and merge this one.

The .nox.yaml change itself is 9 insertions, 0 deletions, YAML-validated with
a duplicate-key-rejecting loader. It is not what is red.

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