Code Review: 23 issues found (7 high)

[prudhvi1709]

Summary: 23 issues found: 5 high, 8 medium, 6 low, 4 info.

Severity	File	Line	Issue	Suggestion
high	public/aipipe.js	L4	Sensitive credentials (token, email) are stored in localStorage, which is accessible to any JavaScript on the page and vulnerable to XSS theft.	Use httpOnly cookies managed server-side to store tokens, preventing JavaScript access entirely.
high	public/login.js	L36	The redirect URL origin check only partially mitigates open redirect: an attacker could register a same-origin path that itself redirects externally, and the token is appended as a plain URL query parameter.	Avoid passing the token in the URL at all; instead use a short-lived server-side session or postMessage to the redirect target.
high	public/login.js	L14	The Google credential (id_token) is passed as a plain query parameter to the /token endpoint, exposing it in server logs, browser history, and referrer headers.	Send the credential in the POST body or as an Authorization header rather than as a URL query parameter.
high	public/aipipe.js	L6	URL parameters prefixed with 'aipipe_' (including the token) are parsed and persisted from any URL, allowing token injection via a crafted link (token fixation / open credential injection).	Only accept and store credentials that arrive via the trusted login redirect flow, not from arbitrary URL parameters.
high	src/utils.js	L25	createToken produces a JWT with no expiry (no setExpirationTime call), meaning issued tokens are valid indefinitely.	Add .setExpirationTime('24h') (or an appropriate short TTL) to the SignJWT chain so tokens expire.
high	src/worker.js	L47	The proxyRequest function is called for the 'proxy' provider without any authentication or authorization check, potentially allowing unauthenticated requests to be proxied.	Apply the same JWT validation and budget checks to the proxy provider as are applied to all other providers.
high	src/worker.js	L43	The 'proxy' provider is reachable before any token validation occurs, bypassing all authentication middleware.	Move the proxy handler to after the token validation block so it requires a valid JWT.
medium	src/utils.js	L17	CORS is set to allow all origins ('*') globally, including for sensitive endpoints like /token and /admin, which should be restricted.	Restrict the Access-Control-Allow-Origin header to trusted origins for authentication and admin endpoints.
medium	src/worker.js	L35	isNativeApiKey detects native keys only by prefix ('sk-' or 'AIza'), which means a crafted JWT starting with those strings would bypass all JWT validation and cost tracking.	Validate the token as a JWT first; only fall back to native-key logic if JWT parsing definitively fails, rather than relying on prefix heuristics.
medium	src/cost.js	L22	The add() method uses INSERT OR REPLACE which deletes and reinserts the row, creating a race condition under concurrent requests where cost increments can be lost.	Use INSERT ... ON CONFLICT DO UPDATE SET cost = cost + ? (an atomic upsert) to prevent concurrent update races.
medium	src/cost.js	L36	The SQL query in cost() includes an ORDER BY clause, which is meaningless on an aggregate query and indicates a copy-paste error.	Remove the ORDER BY clause from the aggregating cost() query.
medium	public/playground.js	L7	If the token is missing, window.location is assigned but code execution continues past that line, potentially causing errors on subsequent DOM operations.	Add a return statement or guard clause after the redirect assignment to prevent further execution.
medium	public/usage.js	L6	If the fetch to the usage endpoint fails or returns a non-JSON response, the destructuring assignment will throw an unhandled error with no user-facing feedback.	Add try/catch error handling around the fetch and render an error message in the $usage element on failure.
medium	src/config.example.js	L1	The example config contains a real-looking Google OAuth client ID in login.js (line 9), and the example salt file ships with placeholder values that may be used as-is in production.	Ensure the build or deployment process validates that config.js exists and contains non-example values before allowing production use.
medium	tests/admin.test.js	—	Admin tests use a shared seedUsage helper that may leave state between tests, causing ordering-dependent test failures.	Isolate each test's storage state with per-test setup/teardown (beforeEach/afterEach) to ensure test independence.
low	public/login.html	L35	The token input field uses type='text' instead of type='password', causing the token to be visible in the browser UI and potentially captured by autofill or password managers incorrectly.	Use type='password' for the token field, or mask the value by default with a show/hide toggle.
low	public/aipipe.js	L8	URLSearchParams.delete() is called with two arguments (key and value), but the second argument is not part of the standard URLSearchParams API in all environments and may silently fail.	Use params.delete(key) with only the key argument for broad compatibility.
low	src/providers.js	L55	tokenCost returns 0 via the
low	public/index.html	L29	External CDN resources (Bootstrap, highlight.js) are loaded without Subresource Integrity (SRI) hashes, making the app vulnerable to CDN-supply-chain attacks.	Add integrity and crossorigin attributes with the appropriate SRI hash to each external stylesheet and script tag.
low	public/login.html	L55	The Google GSI client script is loaded from an external domain without an SRI hash, and a compromised version could steal credentials.	Host a pinned version of the GSI client or add an SRI hash; at minimum add a Content-Security-Policy header restricting script sources.
low	public/index.html	—	No Content-Security-Policy meta tag is present in the HTML pages, leaving the application exposed to XSS escalation.	Add a strict Content-Security-Policy header or meta tag restricting script-src, object-src, and base-uri.
info	tests/authentication.test.js	—	There is no test covering the behavior when a token has expired (e.g., a previously valid JWT past its expiry), which is an important authentication edge case.	Add a test that creates a token with a past expiry time and verifies the worker returns a 401 response.
info	src/providers.js	—	The file is truncated mid-function (openai cost handler cut off at 'asy'), suggesting incomplete code that would cause a syntax/runtime error.	Ensure the full providers.js source is reviewed and that the truncated openai cost async function is complete and correct.

---

Generated by Software Engineer Intern

FYI: Just checking Intern :)