Skip to content

ci: run tests/security/ on pull requests - #533

Open
craig-dt wants to merge 1 commit into
Vigil-SOC:mainfrom
craig-dt:ci/531-run-security-tests-in-pr-ci
Open

ci: run tests/security/ on pull requests#533
craig-dt wants to merge 1 commit into
Vigil-SOC:mainfrom
craig-dt:ci/531-run-security-tests-in-pr-ci

Conversation

@craig-dt

@craig-dt craig-dt commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What

Adds a Run security tests step to the existing test-unit-backend job so tests/security/ runs on every pull request.

Why

tests/security/ holds 84 tests across 4 files — including the deny-by-default assertions added after the 2026-05 disclosure — and no pull-request job ran any of them. ci-cd.yml runs tests/unit/ and tests/integration/ only, both path-scoped, so the whole directory was visible solely to nightly.yml (pytest tests/, 02:00 UTC) — after a regression had already merged.

File Tests Asserts
test_unauth_endpoints.py 40 unauthenticated requests to the disclosure's routes return 401/403; authenticated non-admins get 403
test_path_traversal.py 22 integration IDs reject ../ traversal
test_ssrf.py 21 provider URLs reject private/link-local targets
test_route_auth_coverage.py 1 every /api/* route requires auth or is on PUBLIC_API_PATHS

Decisions worth reviewing

Its own step, not a widened path. Running pytest tests/security/ separately rather than extending pytest tests/unit/ keeps a security failure legible in the job log instead of buried among ~1000 unit tests.

DEV_MODE and JWT_SECRET_KEY are explicit, not defensive. backend/middleware/auth.py:24 defaults DEV_MODE to false, and with no .env in CI an unset JWT_SECRET_KEY makes _load_jwt_secret() raise. test_route_auth_coverage.py never sets it — that file currently only works because test_unauth_endpoints.py calls os.environ.setdefault("JWT_SECRET_KEY", ...) at collection time while the coverage test imports backend.main lazily inside its test body. CI must not depend on that accidental cross-file ordering.

--no-cov. Stops the step from overwriting the coverage.xml that the Codecov step immediately below uploads.

Scope

Test content is unchanged; this only makes existing tests run. Nothing else in CI changes.

Note that test_route_auth_coverage.py is itself vacuous under FastAPI 0.137.1 — it examines 1 route out of 359 and passes unconditionally — so it will pass without asserting anything until #532 lands. The other 83 assertions become real immediately, which is why this is worth landing on its own.

For the record, the auth boundary is currently intact: 359 effective /api/* routes, 351 requiring auth, 7 un-authed and all 7 on PUBLIC_API_PATHS, 0 unprotected. This restores regression protection; it is not a live-vulnerability fix.

Testing

$ env TESTING=true DEV_MODE=false JWT_SECRET_KEY=ci-only-not-a-real-secret \
    pytest tests/security/ -v --no-cov
84 passed, 5 warnings in 7.64s

Also verified: the workflow YAML parses, the new step lands between Run unit tests and Upload coverage to Codecov, and --no-cov leaves coverage.xml untouched.

Clean-environment behaviour (no .env) is proven by this PR's own CI run — that could not be verified locally without moving the developer's .env.

One incidental change: the blank line above the new step previously held trailing whitespace and is now genuinely blank.

Closes #531

🤖 Generated with Claude Code

tests/security/ holds 84 tests across 4 files, including the
deny-by-default assertions added after the 2026-05 disclosure, and no
pull-request job ran any of them. ci-cd.yml runs tests/unit/ and
tests/integration/ only -- both path-scoped -- so the directory was
visible solely to nightly.yml (pytest tests/, 02:00 UTC), i.e. after a
regression had already merged.

Add it as its own step in test-unit-backend rather than widening the
unit path, so a security failure is legible instead of buried in ~1000
unit tests.

DEV_MODE and JWT_SECRET_KEY are set explicitly because
backend/middleware/auth.py defaults DEV_MODE to false, and with no .env
in CI an unset JWT_SECRET_KEY makes _load_jwt_secret() raise.
test_route_auth_coverage.py does not set it itself -- it currently only
works because test_unauth_endpoints.py calls os.environ.setdefault at
collection time. CI must not depend on that ordering.

--no-cov keeps the step from overwriting the coverage.xml that the
Codecov step uploads.

Test content is unchanged. Note that test_route_auth_coverage.py is
itself vacuous under FastAPI 0.137.1 and will pass without asserting
anything until Vigil-SOC#532 lands; the other 83 assertions become real
immediately.

Closes Vigil-SOC#531

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Craig <craig@deeptempo.ai>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

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.

tests/security/ never runs in PR CI (84 security tests outside the merge gate)

1 participant