test: add auth unit tests and wire frontend into CI - #13
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012RttwcxBgsnXH2TwQGQsrp
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.
What
tests/test_auth.py— 20 unit/integration tests coveringapi/auth.py's current behavior.api/auth.pyitself is NOT modified.frontendjob into.github/workflows/ci.yml(npm ci→npm run build→npm test, matchingpackage.json's actual script names), and makes thedockerjob depend on it too.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) intest_require_auth_unset_secret_accepts_token_forged_with_empty_secret:When
AUTH_ENABLED=truebutJWT_SECRETis unset/empty,require_auth()callsvalidate_token(token, "")— i.e. it verifies the JWT signature against an empty-string HMAC secret. PyJWT does not reject an empty secret for HS256:So anyone can forge a token signed with
secret=""and it passes validation — auth is effectively bypassable whenever a deployment setsAUTH_ENABLED=truewithout also settingJWT_SECRET. The test confirms/protectedreturns200 {"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 whenAUTH_ENABLED=trueandJWT_SECRETis 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'sx_devhub_internalparam name does the actual header binding via FastAPI'sHeader(). Dead code, not a bug.)Test coverage (
tests/test_auth.py)extract_token(): valid, missing header, missingBearerprefix, wrong scheme.validate_token(): valid, expired, malformed/garbage, wrong secret.require_auth()end-to-end (via a throwawayFastAPIapp +TestClient, exercising real header parsing):AUTH_ENABLEDunset/false → no-op ("system"), and confirmed this doesn't accidentally depend on/validateX-Devhub-Internal's value (still no-ops even with a wrong internal-header value present).Authorizationheader rejected (401) whenAUTH_ENABLED=true.X-Devhub-Internalbypass: 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_SECRETunset: internal-header path proven unreachable (falsy secret short-circuits theand); and the empty-secret forgery gap above.sub/service/username/email) falls back to"unknown".api/auth.pycoverage: before → afterFull pytest results
Frontend CI job + local simulation
Added a
frontendjob to.github/workflows/ci.ymlrunning insideomnibioai-dev-hub-ui/:npm ci→npm run build→npm test(checkedpackage.jsonfirst —"build": "tsc && vite build","test": "vitest run"). Thedockerjob'sneedsnow includesfrontendalongsidelint-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'):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.pyuntouched.Per instructions, not merged automatically. Results reported above; merge only after explicit confirmation.