Skip to content

test: add auth unit tests and wire frontend into CI - #13

Merged
man4ish merged 1 commit into
mainfrom
test/auth-and-frontend-ci
Sep 1, 2026
Merged

test: add auth unit tests and wire frontend into CI#13
man4ish merged 1 commit into
mainfrom
test/auth-and-frontend-ci

Conversation

@man4ish

@man4ish man4ish commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

What

  1. Adds tests/test_auth.py — 20 unit/integration tests covering api/auth.py's current behavior. api/auth.py itself is NOT modified.
  2. Wires a frontend job into .github/workflows/ci.yml (npm cinpm run buildnpm test, matching package.json's actual script names), and makes the docker job depend on it too.

⚠️ Security gap found in api/auth.py — reported, NOT fixed

Per instructions, this is a test-only PR — I did not touch api/auth.py. But writing the JWT_SECRET-unset tests surfaced a real bug, documented (not fixed) in test_require_auth_unset_secret_accepts_token_forged_with_empty_secret:

When AUTH_ENABLED=true but JWT_SECRET is unset/empty, require_auth() calls validate_token(token, "") — i.e. it verifies the JWT signature against an empty-string HMAC secret. PyJWT does not reject an empty secret for HS256:

>>> import jwt
>>> jwt.encode({"sub": "attacker"}, "", algorithm="HS256")
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhdHRhY2tlciJ9.OHV_4LqxloNHowlMkpndXpnhke6PDMwmT8QsPu4rLjY'
>>> jwt.decode(_, "", algorithms=["HS256"])
{'sub': 'attacker'}

So anyone can forge a token signed with secret="" and it passes validation — auth is effectively bypassable whenever a deployment sets AUTH_ENABLED=true without also setting JWT_SECRET. The test confirms /protected returns 200 {"actor": "attacker"} for a self-forged, unsigned-in-any-meaningful-sense token, instead of failing closed.

Suggested direction (not implemented here): require_auth() (or _jwt_secret()) should raise a 500/fail closed when AUTH_ENABLED=true and JWT_SECRET is unset/empty, rather than silently falling through to a forgeable empty-secret JWT check. Flagging for a separate fix PR — let me know if you want me to open one.

(Secondary, non-security observation: _INTERNAL_HEADER = "x-devhub-internal" at api/auth.py:49 is defined but never referenced — require_auth's x_devhub_internal param name does the actual header binding via FastAPI's Header(). Dead code, not a bug.)


Test coverage (tests/test_auth.py)

  • extract_token(): valid, missing header, missing Bearer prefix, wrong scheme.
  • validate_token(): valid, expired, malformed/garbage, wrong secret.
  • require_auth() end-to-end (via a throwaway FastAPI app + TestClient, exercising real header parsing):
    • AUTH_ENABLED unset/false → no-op ("system"), and confirmed this doesn't accidentally depend on/validate X-Devhub-Internal's value (still no-ops even with a wrong internal-header value present).
    • Valid JWT accepted; expired JWT rejected (401); garbage token rejected (401); missing Authorization header rejected (401) when AUTH_ENABLED=true.
    • X-Devhub-Internal bypass: correct value → "devhub-ui", no JWT needed; header absent → falls through to JWT path; header present with wrong value → falls through and fails (401).
    • JWT_SECRET unset: internal-header path proven unreachable (falsy secret short-circuits the and); and the empty-secret forgery gap above.
    • Bonus: JWT with no identity claim (sub/service/username/email) falls back to "unknown".

api/auth.py coverage: before → after

Statements Missed Coverage
Before (full suite minus test_auth.py) 43 28 35%
After (full suite incl. test_auth.py) 43 0 100%

Full pytest results

  • Before this PR (baseline): 175 passed
  • After this PR: 195 passed (175 + 20 new auth tests), no regressions

Frontend CI job + local simulation

Added a frontend job to .github/workflows/ci.yml running inside omnibioai-dev-hub-ui/: npm cinpm run buildnpm test (checked package.json first — "build": "tsc && vite build", "test": "vitest run"). The docker job's needs now includes frontend alongside lint-and-test.

Ran all three steps locally against the current frontend code, before ever running in CI (Node v20.20.2, matching the job's node-version: '20'):

$ npm ci
added 357 packages, and audited 358 packages in 2s
(11 known vulnerabilities in transitive deps — pre-existing, unrelated to this change)

$ npm run build
> tsc && vite build
✓ 201 modules transformed.
dist/index.html                   0.51 kB
dist/assets/index-CsOyjkH6.css   14.32 kB
dist/assets/index-Ov1nyYn2.js   280.30 kB
✓ built in 704ms

$ npm test
> vitest run
 Test Files  11 passed (11)
      Tests  25 passed (25)

All three steps pass cleanly on the current frontend code. This CI job will go green, not red, on first run.

Scope

2 files: .github/workflows/ci.yml, tests/test_auth.py (new). api/auth.py untouched.

⚠️ Do not auto-merge

Per instructions, not merged automatically. Results reported above; merge only after explicit confirmation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RttwcxBgsnXH2TwQGQsrp
@man4ish
man4ish merged commit 2f7ffcc into main Sep 1, 2026
1 of 2 checks passed
@man4ish
man4ish deleted the test/auth-and-frontend-ci branch September 1, 2026 01:36
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.

1 participant