test(gateway): cover malformed Authorization header on /api/config#45
Conversation
There was a problem hiding this comment.
Code Review — PR #45
Repository context: ShellClaw is a C99/C11 edge AI agent (Jetson / Raspberry Pi) built with make, organized into src/core, providers, tools, channels, gateway, asap, sandbox, and hardware. Gateway auth is centralized in http_lws.c via http_request_bearer_token() and the requires_auth() gate on /api/* routes.
Summary
Focused, test-only PR that closes a real coverage gap: /api/config must return 401 when an Authorization header is present but not in Bearer <token> form. The assertion matches production behavior in http_request_bearer_token() (src/gateway/http_lws.c:127–128) and the auth gate (http_lws.c:206–210). No production code changes; scope is appropriate and the cherry-pick from #42 is well-scoped.
Must Fix
None.
Should Fix
- DRY / helper reuse (
tests/test_gateway_http.c:366–393): The new test inlines a full libcurl setup. Existing helpershttp_get()andhttp_get_auth()cover the no-auth and valid-Bearercases but not arbitrary header values. Consider a smallhttp_get_header(url, "Authorization: …", …)helper so future malformed-header cases do not copy ~25 lines of curl boilerplate. - Timeout inconsistency (
tests/test_gateway_http.c:380): UsesCURLOPT_TIMEOUT, 10Lwhile sibling helpers use5L. Not wrong, but inconsistent without a documented reason.
Nice to Have
- Assert the response body contains
{"error":"Unauthorized"}(returned byhttp_lws.c:208) to lock the contract, not only the status code — consistent with how positive-path tests assert JSON fields. - Additional malformed inputs on the same code path:
Bearer(no space/token),bearer <token>(lowercase scheme — implementation uses case-sensitivestrncmp("Bearer ", …)per RFC 6750 this is acceptable fail-closed behavior, but worth a one-line comment or extra case if you want explicit regression coverage). - One endpoint is sufficient because auth is centralized; no need to duplicate across every
/api/*route unless you want belt-and-suspenders documentation in the test file.
Positive Highlights
- Fills the matrix between
test_api_config_401(no header) andtest_api_config_invalid_bearer(wrong token) — exactly the edge casehttp_request_bearer_tokenis written to reject. - Correctly placed in
main()before pairing (tests/test_gateway_http.c:1038–1040); no token setup required. - PR description is clear about superseding #42 and skipping the duplicate invalid-bearer test already on
development.
Verdict: Approve — safe to merge as-is; suggestions above are polish only.
Sent by Cursor Automation: Adrianno’s personal code review


Summary
Cherry-picks the unique test value from PR #42 into
development.test_api_config_401_malformed_auth— verifies/api/configreturns 401 when theAuthorizationheader is present but notBearer <token>format.test_api_config_401_invalid_bearerfrom test(gateway): cover invalid and malformed bearer auth on /api/config #42;test_api_config_invalid_beareralready exists ondevelopment.Supersedes the test portion of #42 (which targeted
main). Close #42 after merge.Test plan
GATEWAY=1 make test_gateway_http— all tests passed locally