Security hardening rollup: verify OIDC JWTs, gate every route, Next 16 — supersedes #78/#79/#80

[maxsogin]

Security hardening rollup — supersedes #78, #79, #80

Fixes the security findings from the Aikido feed (114 issues) and the manual
review in security-review/. Nine commits, each independently reviewable and
revertable. Replaces PRs #78, #79 and #80 — see "Why those are superseded"
below; #78 in particular should not be merged.

Results

	Before	After
npm audit --omit=dev	2 critical, 11 high	0 vulnerabilities
npm audit (incl. dev)	21 total	1 low (esbuild, Windows-only dev server)
Aikido local scan (36 files)	18 findings	9, all previously-verified false positives
Jest	1844 passing / 184 suites	1936 passing / 189 suites
tsc --noEmit	clean	clean
next build	green	green (Turbopack, now the Next 16 default)

Commits

Commit	What
security(deps)	next 15.1.3 → 16.3.8, mysql2 → 3.24.5, sharp → 0.35.5, uuid → 11.1.1, csv-parse removed
security(ci)	Least-privilege workflow tokens, correct action SHA pins, no script injection
security(auth)	Verify the OIDC JWT signature instead of trusting the header
security(authz)	Deny-by-default proxy.ts gate + ALLOWED_ORGS allowlist
security(injection)	JQL, Jira REST path, 4 XSS sinks, CSV formula injection
security(disclosure)	/api/llm-config secrets, /api/debug/headers deleted, generic errors
security(runtime)	Non-root container, supported Node, security headers
security(logs)	logSafe() log-injection guard, prompt-loader containment
security(auth) #2	Pin JWT algorithms on the JWKS branch too

The three that matter most

1. The identity header was never verified. extractUser() split
x-amzn-oidc-data on ., base64-decoded segment one, and trusted email and
groups verbatim — no signature, no expiry, no issuer. That header is an
ordinary HTTP header once it leaves the load balancer, so anything able to reach
the app port was whichever identity, in whichever groups, it claimed:

PAYLOAD=$(printf '{"email":"x@y","groups":["splunk-admin"]}' | base64)
curl -X POST http://app:3000/api/report -H "x-amzn-oidc-data: e30.$PAYLOAD.sig"

That forged header satisfied requireAdmin() and unlocked report deletion,
schedule CRUD, Jira issue transitions and due-date writes, vulnerability syncs,
and — because buildCostVisibility() short-circuits for admins — every
developer's Claude spend. AWS documents signature verification as mandatory for
this header for exactly this reason.

Now verified with jose: algorithm pinned from our side (so alg: none and HMAC
substitution are rejected rather than read out of the token), exp/nbf
enforced, iss enforced when configured, and two key modes —
AUTH_ALB_REGION (ALB serves one PEM per kid; it publishes no JWKS document)
or AUTH_JWKS_URL. Enabled-but-unconfigured denies every request rather than
falling back to trusting the header.

2. Authorization was opt-in per handler. 24 of 62 exported handlers
referenced no auth function at all. Reachable with nothing but network access:
/api/vulnerabilities/alerts (the org's unpatched critical CVEs with SLA due
dates — a prioritised target list), every /api/report/* read,
/api/settings/user-mappings (the GitHub↔Jira identity table), POST /api/chat
(an LLM agent over seven DB tools), and /api/developers?source=github plus
POST /api/settings/github/test-connection, which both proxy the server's
GitHub PAT at a caller-chosen org.

Two of the vulnerability route files even carried the comment "Readable by every
signed-in user … Do not add requireAdmin" — documenting a signed-in check that
nothing in the request path performed.

src/proxy.ts now requires a verified identity for everything except
/api/health. requireAdmin stays in the handlers: the gate establishes
authentication, handlers enforce authorization. The matcher is deliberately broad
so a new route is covered by default rather than needing to be remembered.

3. Stored XSS in four places. The dev summary, team pulse and two chat-panel
sites pushed LLM narrative into dangerouslySetInnerHTML after a few markdown
regexes, escaping nothing. That text derives from commit messages, PR titles,
branch names and Jira summaries — writable by anyone who can push to a scanned
repo — and the dev summary is persisted to developer_summaries, so one
injection was replayed to every later viewer including admins, with no CSP to
contain it and no CSRF token to stop the resulting script driving admin routes.
All four now use one SafeMarkdown component that returns React elements, so
text nodes are escaped by React.

Why #78, #79 and #80 are superseded

#78 — do not merge. Three independent problems:

• Its lockfile recorded integrity: sha512-PiZLhL… for next@15.2.0, which
  matches neither 15.2.0 (sha512-VaiM7s…) nor 15.1.3 (sha512-5igmb8…). The
  hash was not produced by an install, so npm ci aborts with EINTEGRITY
  and the Docker build cannot run. Its own CI confirms this.
• It does not fix the CVE in its title. The React-flight RCE is patched on the
  15.2 line only at 15.2.6; the affected range is
  >=15.2.0-canary.0 <15.2.6, so 15.2.0 is inside it.
• It leaves 4 criticals and 12 highs open, and adds two highs whose affected
  ranges begin at 15.2.0 (App Router proxy bypass via segment-prefetch), which
  15.1.3 was not subject to. 15.x also reaches end of security support on
  2026-10-21 regardless.

This PR goes to 16.3.8 (Active LTS, supported to at least Oct 2027) via a real
npm install, so every integrity hash is the one the registry serves. Migration
surface was verified nil before upgrading: no next/image, no next/headers,
all dynamic route params already Promise-typed, no parallel routes, no custom
webpack config, no next lint, no next/cache.

#79 — right idea, two wrong pins. actions/checkout@11bd719… is tagged
v4.2.2 but commented # v5.2.2; actions/setup-node@39370e39… is tagged
v4.1.0 but commented # v5.1.1. Neither claimed tag exists. It silently
downgraded both actions across a major version while labelling them v5 — and a
false version comment defeats the one maintainability benefit pinning has. All
five pins here were resolved from the upstream tag refs and verified against
their comments.

#80 — correct but incomplete. test.yml still had no permissions: block at
all (that's the workflow which runs on pull_request, i.e. the fork-PR path);
build-and-push's checkout still persisted credentials; and the
${{ github.ref }} script injection was untouched.

Scope notes — what this deliberately does not do

Per-user tenancy. ALLOWED_ORGS closes the credentialed-GitHub-proxy problem
outright and bounds cross-tenant reads to orgs an operator deliberately hosts
together. Deciding which orgs a particular user may see needs a membership
model (an OIDC group claim or an explicit table) and is a product decision, not
something to invent in a security fix. Report lookups are still keyed on a global
report id.

Rate limiting. Several endpoints still do expensive LLM work on demand and
accept a caller-supplied refresh=true. Fixing properly needs a token-bucket
store and a budget policy.

New / changed configuration

Variable	Effect
AUTH_ALB_REGION or AUTH_JWKS_URL	Required when AUTH_ENABLED=true — how the token signature is verified. Without one, all requests are denied.
AUTH_EXPECTED_ISS	Optional; pins the iss claim.
AUTH_JWKS_ALGS	Optional; default RS256,ES256.
AUTH_ALLOW_ANONYMOUS_ADMIN	Required to open admin routes when AUTH_ENABLED=false in production.
ALLOWED_ORGS	Recommended. Unset accepts any well-shaped org name (preserves existing installs); validateEnv warns.
AUTH_TEST_ALLOW_IN_PRODUCTION	Removed. It made a full authentication bypass a supported configuration, and docker-compose.yml passed it through.
DB_USER, DB_PASSWORD, MYSQL_ROOT_PASSWORD	Now required by docker-compose.yml (were hardcoded literals).

AUTH_ENABLED unset now means enabled in production. The old === 'true'
test let a dropped env var silently turn every authorization check into "allow".

Verification still needed

Could not be exercised from my environment:

• CI going green. better-sqlite3 has no prebuilt binding for the local
  Node 26 ABI, so 13 of 202 suites fail locally on that alone — identical to the
  pre-change baseline, and expected to pass on CI's Node 22.
• docker run --rm <image> id returning uid=1000. No container runtime
  available locally.
• The CSP against real traffic. script-src 'self' with no unsafe-inline
  is the directive that blocks an injected handler; unsafe-inline remains in
  style-src because Recharts and chart.tsx's CSS-variable block need it, and
  img-src allows avatars.githubusercontent.com. Consider deploying as
  Content-Security-Policy-Report-Only first.

Note on the test suite

Five suites asserted the vulnerable behaviour as expected. auth.test.ts built
tokens signed with the literal string "fakesignature" and asserted
extractUser accepted them; four others asserted that Jira API error (403)
and xxxxx12345 reach the client. All are rewritten as regression guards rather
than deleted.

[msogin]

Pushed 6 commits on top of db52f92 after testing this locally (podman + headless Chrome) and against dev's setup. Summary:

Merge with main (849d459, 2ceac8b) — resolves the conflicts with GLOOK-59. The new freshness fields on /api/llm-config go to every viewer (timestamps/booleans only), and the admin/public config split is kept. GLOOK-59's report route called extractUser() without await, which no longer type-checks now that it's async.

Blocker 1 — the CSP broke the app in a real browser (08825d6). A static script-src 'self' blocks Next's inline hydration scripts (self.__next_f.push(...)). Every page rendered as static HTML with no data, and React threw error #412. curl can't see this. Now proxy.ts mints a 128-bit nonce per request (script-src 'self' 'nonce-…' 'strict-dynamic'), and the root layout renders per request. Verified in Chrome: 0 violations, all pages render data, and an injected inline script without the nonce is still blocked. 401s from the gate now get default-src 'none' (they had no CSP).

Blocker 2 — browser vs MCP on dev (23ffcd6). Dev has two identity paths, and this PR verified only one. The mcp-okta-proxy sidecar sent an unsigned alg=none JWT in x-amzn-oidc-data, which verification correctly rejects, so either the browser or MCP got locked out. Fix:

• The sidecar now forwards the caller's original, validated Okta token (Smartling/mcp-okta-proxy#23, OKTA_MCP_PROXY_ACCESS_TOKEN_HEADER_NAME).
• Glooker verifies it against Okta's JWKS (RS256, iss, aud, exp, optional cid) and takes email/groups from /userinfo with that token.
• It's accepted only on /api/mcp: proxy.ts strips the header everywhere else.
• Config: AUTH_OKTA_ISSUER + AUTH_OKTA_AUDIENCE (both are required; validateEnv errors on half-config).

Next 16 regression — boot migrations deadlocked (2932f8b). The db module was instantiated more than once per process, so the boot ALTERs ran concurrently: 8 "Deadlock found" on a local boot, versus 0 on main under the same load. On a new column that would leave it silently missing. Fixed with a globalThis singleton, like the other stores. The regression test fails on the old code.

Compose (280b2a4): pass AUTH_ALLOW_ANONYMOUS_ADMIN and ALLOWED_ORGS through, needed for a local production build.

Jest: 2177/2178 locally. The one failure comes from untracked duplicate files on my machine and passes without them. tsc clean, next build OK.

Still open here: non-admins lose the Projects nav link and Jira host links, because the public config drops jira (NavBar.tsx reads config.jira.enabled/projectsJql).

Dev rollout needs (glooker-deploy):

• App: AUTH_ALB_REGION=us-east-1, ALLOWED_ORGS=Smartling, AUTH_OKTA_ISSUER/AUTH_OKTA_AUDIENCE=https://smartling-login.okta.com.
• Sidecar: image 0.2.17 with OKTA_MCP_PROXY_ACCESS_TOKEN_HEADER_NAME=x-okta-access-token, and drop USERINFO_HEADER_NAME.
• ECS host: /var/log/glooker writable by uid 1000 (the container is now non-root).

[msogin]

Smartling review

Assessment: Ready to merge with fixes. The PR closes real holes (unsigned JWT trust, ungated routes, CSV/formula injection, the org override in chat). However, the ALB path still lets anyone with their own ALB in the same region forge identities until signer and the issuer are pinned, and the JWKS path should pin the audience.

The inline comments below cover the specific issues. Cross-cutting recommendations:

• Authentication is now enforced only in the proxy and its matcher. Add a test that asserts the matcher covers /api/* and page routes.
• Add negative tests to proxy-auth-gate.test.ts and auth.test.ts for wrong audience, wrong issuer, and a foreign ALB signer.
• src/app/api/chat/route.ts: req.json() sits outside the try block, so a malformed body returns an unhandled 500. Move it inside the try.

[msogin]

🔴 critical — ALB tokens are accepted without checking the signer header against our ALB ARN. The public-keys.auth.elb.<region>.amazonaws.com key host serves keys for every ALB in the region, so anyone running their own ALB and OIDC IdP in the same region can mint an ES256 token with any email/groups, including admin. That is a full authentication bypass. AUTH_EXPECTED_ISS is also optional, so the issuer is not pinned by default.

Fix: add a required AUTH_ALB_ARN, read signer from decodeProtectedHeader, and reject unless it equals that ARN. Make AUTH_EXPECTED_ISS mandatory on the ALB path, and fail closed in env-validation when either is missing.

[msogin]

Fixed in b73b163. The ALB path now requires AUTH_ALB_ARN and rejects any token whose signer header differs. That check runs before any key fetch, so a foreign token costs no outbound request either. AUTH_EXPECTED_ISS is now mandatory on the ALB path, and validateEnv errors when either is missing. Tests: a correctly signed token from another ALB is rejected with zero fetches, a token with no signer is rejected, and a missing ARN or issuer denies everything. The foreign-signer test fails if the check is removed. Dev values: issuer https://smartling-login.okta.com (the ALB's authenticate_oidc issuer); the ARN is derivable from the listener ARN terraform already reads.

[msogin]

🟡 warning — The JWKS path checks no audience, and the issuer is only checked when AUTH_EXPECTED_ISS is set. Any token the same IdP signed for a different client app would authenticate here (token confusion across apps).

Fix: add AUTH_EXPECTED_AUD, pass audience to jwtVerify, and require both issuer and audience whenever AUTH_JWKS_URL is used.

[msogin]

Fixed in b73b163. The JWKS path requires AUTH_EXPECTED_ISS and the new AUTH_EXPECTED_AUD (passed as audience to jwtVerify); without either, every token is denied and validateEnv errors. Tests: wrong audience (a token for another app), wrong issuer, and each pin missing.

[msogin]

🟡 warning — Every token with a new, well-formed kid triggers an outbound fetch to AWS, and failed lookups are not cached. albKeyCache also grows without limit. Unauthenticated callers can use this to make the app send unlimited outbound requests (amplification).

Fix: cache misses for a short time, share one in-flight lookup per kid, and bound the cache size.

[msogin]

Fixed in b73b163. Key lookups are bounded (ALB_KEY_CACHE_MAX = 100, FIFO). Misses are cached for 60s, and concurrent lookups for the same kid share one in-flight fetch. The signer pin above also runs first, so only tokens claiming our ALB reach a lookup at all. Tests cover each of these.

[msogin]

🟡 warning — The token is verified here, then again in every handler (requireAdmin, resolveRequester, /api/auth/me). That is two or more signature checks per request, and on the ALB path a cold key cache means duplicate key fetches.

Fix: have the proxy strip an internal identity header from inbound requests, set it with the verified identity, and have handlers read that header. Or memoize verification per request.

[msogin]

Addressed in b73b163 with a per-token memo instead of an internal header. A token that fully verified maps to its identity until its own exp (at most 5 min, bounded to 1,000 entries), so the gate and the handlers verify once, and the Okta path calls /userinfo once. I avoided the internal trusted header on purpose: if the proxy matcher ever missed a path, a client-supplied copy would be trusted, which is the class of bug this PR removes. The memo is only written after full verification, so it can't admit anything verification wouldn't. Tests: jwtVerify runs once across three calls, and a memoized identity is not reused past token expiry.

[msogin]

🟡 warning — The /userinfo response is trusted without checking that info.sub matches the token's sub. If AUTH_OKTA_USERINFO_URL is misconfigured, the identity could come from a different principal.

Fix: reject when info.sub !== payload.sub.

[msogin]

Fixed in b73b163, with one Okta-specific detail. For org-server access tokens, sub is the user's login and the Okta user id is in uid, while /userinfo returns sub = the user id. So the check is info.sub === (payload.uid ?? payload.sub), and a missing info.sub is rejected too. Tests: a different principal is rejected, the org-server uid shape is accepted, and a missing sub is rejected.

[msogin]

🔵 suggestion — In production, AUTH_ENABLED=false alone opens every read route; AUTH_ALLOW_ANONYMOUS_ADMIN only controls admin rights.

Fix: in production, also require a second explicit opt-in (e.g. AUTH_ALLOW_ANONYMOUS=true) before letting requests through, or refuse to start.

[msogin]

Fixed in b73b163. In production, AUTH_ENABLED=false now also needs AUTH_ALLOW_ANONYMOUS=true; otherwise the gate answers 503 for everything except /api/health (the liveness probe stays up), and validateEnv errors. Outside production nothing changes. Compose passes it through, and the local podman flow sets it explicitly.

[msogin]

🔵 suggestion — The cid pin is only enforced when AUTH_OKTA_CLIENT_ID is set.

Fix: make it required, or document that it is effectively required whenever one audience is shared by several clients.

[msogin]

Made it required in b73b163. Dev's audience is the org-wide https://smartling-login.okta.com, which every app in the tenant shares, so without the cid pin another app's token would pass. With issuer and audience set but no client id, Okta tokens are denied and validateEnv errors. Test added.

[msogin]

Review-loop (2 personas, 1 loop)

Reviewers: Security Engineer + Security and Sr. Backend/Next.js Dev + Correctness. Intent: safe to merge. Verdict: not yet. There are 3 Critical issues: the token is not bound to this app (ALB signer check, JWKS aud/iss), and two Dockerfile runtime issues.

The reviewer also verified locally that tsc is clean, all 2178 jest tests pass, and next build succeeds. Every auth call site is awaited, and padded ALB tokens still verify under jose 6.

Deferred / non-inline:

• Release notes: production with AUTH_ENABLED unset is now gated, and MCP needs AUTH_OKTA_ISSUER/AUTH_OKTA_AUDIENCE. Document the required env vars and the rollout order.
• ALLOWED_ORGS empty means any org is allowed (documented, and a warning is logged). Consider requiring it in production in a follow-up.
• Skipped: there is no per-user org membership (product decision), and /public assets other than the favicon return 401 when unauthenticated (consistent with deny-by-default).

[msogin]

[Critical | Security] ALB mode never checks the signer header. ALB signing keys are regional, not per-LB. Without comparing header.signer to this app's ALB ARN (and with AUTH_EXPECTED_ISS optional), a token minted by an attacker-controlled ALB in the same region verifies and can carry arbitrary email/groups. AWS documents the signer check as required. Suggest: required AUTH_ALB_ARN, reject unless header.signer matches; require issuer in ALB mode.

[msogin]

[Critical | Security] JWKS mode checks neither audience nor issuer by default. issuer is enforced only when AUTH_EXPECTED_ISS is set, and audience is never checked. With a shared IdP JWKS, a token issued for any other client in the tenant is accepted, including its groups. The Okta path pins both, so the two paths are inconsistent. Suggest: required AUTH_EXPECTED_ISS + AUTH_EXPECTED_AUD, fail closed if missing.

[msogin]

[Important] Startup validation misses the new default-on auth and the verifier settings. (a) The missing-verifier hard error fires only when AUTH_ENABLED === 'true', but auth is now also on when it is unset in production, which is the upgrade path for existing deployments. Those boot without an error and then return 401 for every request. The same old check is used at line ~147 for the AUTH_ADMIN_GROUP warning. Use isAuthEnabled() in both places. (b) Also fail at startup when iss/aud/ALB ARN are missing (see auth.ts comments).

[msogin]

[Critical | Correctness] Healthcheck probes 127.0.0.1, but the standalone server binds to $HOSTNAME, which Docker sets to the container ID (the container's IP, not loopback). The probe fails, so containers are marked unhealthy, which can trigger restart loops under ECS or compose service_healthy. Suggest ENV HOSTNAME=0.0.0.0 and PORT=3000 in the runner stage.

[msogin]

[Critical | Correctness] USER node, but /app is still owned by root. Only the copied files are chowned. The default SQLite path is cwd/glooker.db, so the default DB_TYPE fails with EACCES on first DB access. Existing root-owned volumes and LOG_DIR mounts are affected too. Suggest chown node:node /app (or a data dir) before USER node, plus a release note about the volume-ownership migration.

[msogin]

[Important | Correctness] The base-path guard hard-codes the /rest/api/<v>/ prefix. baseUrl keeps any context path in JIRA_HOST (e.g. host/jira), so every jiraFetch fails on self-hosted Server/DC installs with a context path. Suggest comparing against new URL(this.baseUrl).pathname + '/'.

[msogin]

[Ask] Plain docker compose up now returns 401 for everything. AUTH_ENABLED is empty and NODE_ENV=production, so auth is on with no verifier. DB credentials are now required as well. Please confirm this is intended, and document the minimal local env in the README and .env.example.

[msogin]

Review addressed in b73b163. Every inline comment has a reply. Review-level points:

• Matcher coverage: new test in auth.test.ts walks every route.ts / page.tsx under src/app, asserts the proxy matcher matches each one, and asserts it skips only _next/static, _next/image, favicon.ico and icon.svg.
• Negative tests: wrong audience, wrong issuer, foreign ALB signer, missing signer, and each missing pin (ALB and JWKS), plus the Okta cid and /userinfo sub mismatch.
• /api/chat malformed body: now a 400 (request body must be JSON) rather than moving req.json() into the existing try, which would have turned it into a 500. Test added.

npx jest: 2208/2209. The one failure comes from untracked duplicate files on my machine and passes without them. tsc clean; image builds. Redeployed locally: pages and APIs 200, no CSP violations, no migration deadlocks.

Dev config this now requires (on top of the earlier list): AUTH_ALB_ARN (the shared ALB's ARN, derivable from the listener ARN in remote state), AUTH_EXPECTED_ISS=https://smartling-login.okta.com, and AUTH_OKTA_CLIENT_ID (the MCP client id already in SSM at /glooker/okta_client_id).

[msogin]

Fixed the last open item in f1df67f. The public (non-admin) /api/llm-config now includes jira.enabled, jira.host (already visible in every issue link) and a new boolean jira.projectsEnabled. It never includes the JQL, the service-account username or which credentials are missing. NavBar reads projectsEnabled, so viewers keep the Projects link, and developer pages keep their Jira issue links. Tests pin the fields, the absence of each secret, and the NavBar show/hide from a viewer's config. npx jest 2214/2215 (the one local-only failure as before).