Skip to content

Reject platform admin HMAC sessions - #306

Merged
seonghobae merged 1 commit into
masterfrom
fix/hmac-system-admin-session-boundary-20260529
May 29, 2026
Merged

Reject platform admin HMAC sessions#306
seonghobae merged 1 commit into
masterfrom
fix/hmac-system-admin-session-boundary-20260529

Conversation

@seonghobae

@seonghobae seonghobae commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reject system_admin and platform_admin claims on the legacy HMAC fallback session path
  • keep OIDC/JWKS verification authoritative for platform-wide roles
  • update signed bearer regression tests and the accounts boundary test to expect auth failure before mailbox-owner authorization
  • document the anti-pattern in AGENTS and a plan note

Evidence

  • Master Strix run 26641767705 reported JWT Authentication Bypass via HMAC Secret Disclosure, specifically forged system_admin claims when AUTH_SESSION_HMAC_SECRET is known.
  • Tracked files do not include backend/.env; the remaining defensible fix is to reduce HMAC fallback blast radius so platform-wide roles cannot be minted through a symmetric session secret.

Verification

  • PYTHONDONTWRITEBYTECODE=1 python3 -m pytest backend/tests/test_auth_real.py backend/tests/test_accounts_api.py backend/tests/test_observability_api.py backend/tests/test_security_api.py backend/tests/test_data_api.py backend/tests/test_ai_hub_api.py backend/tests/test_tasks_api.py -q
  • PYTHONDONTWRITEBYTECODE=1 python3 -m pytest backend/tests -q
  • PYTHONDONTWRITEBYTECODE=1 python3 -m bandit -r backend/ -x backend/tests/ -q
  • git diff --check

Notes

  • This does not remove tenant-scoped HMAC sessions.
  • Platform-wide roles must come through OIDC/JWKS or a separately audited support flow.

Summary by CodeRabbit

  • Bug Fixes

    • Secured HMAC fallback authentication by preventing system_admin and platform_admin role assignment. Authentication now properly rejects unauthorized claims with HTTP 401. Platform-wide operators must use OIDC/JWKS authentication path.
  • Documentation

    • Added security roadmap documenting JWT authentication hardening measures and anti-patterns to prevent privilege escalation vulnerabilities.

Review Change Stack

@seonghobae
seonghobae temporarily deployed to fix/hmac-system-admin-session-boundary-20260529 - naruon PR #306 May 29, 2026 14:27 — with Render Destroyed
@seonghobae
seonghobae enabled auto-merge May 29, 2026 14:27
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: aeef2162-8fb3-45aa-8c25-cb3d2c445efc

📥 Commits

Reviewing files that changed from the base of the PR and between 6056e0d and 54cc48d.

📒 Files selected for processing (5)
  • AGENTS.md
  • backend/api/auth.py
  • backend/tests/test_accounts_api.py
  • backend/tests/test_auth_real.py
  • docs/plans/2026-05-29-hmac-system-admin-boundary.md

📝 Walkthrough

Walkthrough

This PR tightens JWT authentication security by preventing legacy HMAC-signed sessions from granting platform-wide administrative roles. A new payload validation gate rejects system_admin and platform_admin claims in the HS256 fallback path; downstream tests are updated to expect 401 authentication rejection, and security planning documents are introduced.

Changes

HMAC system-admin boundary enforcement

Layer / File(s) Summary
HMAC system-admin role rejection in legacy auth path
backend/api/auth.py, backend/tests/test_auth_real.py
After HS256 signature verification, the JWT payload is passed through a new _reject_hmac_system_admin_payload guard that denies system-admin and platform-admin role claims, raising HTTP 401. Two new regression tests verify that HMAC sessions bearing these platform-scoped admin roles are rejected.
Downstream accounts API test alignment
backend/tests/test_accounts_api.py
The accounts config endpoint test for system-admin mailbox-owner sessions now expects HTTP 401 "Authentication required" responses instead of 403, reflecting the new auth boundary that rejects such tokens before authorization checks.
Security roadmap and governance anti-pattern recording
docs/plans/2026-05-29-hmac-system-admin-boundary.md, AGENTS.md
A new roadmap document details the critical JWT bypass vector (HMAC secret compromise enabling forged platform admin tokens) and the multi-step remediation plan including boundary enforcement and regression tests. A governance rule is recorded in AGENTS.md forbidding HMAC sessions from authorizing system/platform admin roles.

Possibly related PRs

  • Seongho-Bae/naruon#300: Both PRs implement the same auth-boundary fix by rejecting system_admin/platform_admin roles in the legacy signed/HMAC session flow before accessing /api/accounts/config—main PR via the shared auth payload/role filtering in backend/api/auth.py and updated test_auth_real.py/test_accounts_api.py, and retrieved PR #300 via an accounts-specific guard and corresponding mailbox-owner rejection tests.
  • Seongho-Bae/naruon#231: Both PRs update backend/api/auth.py's legacy HS256/HMAC session token validation to fail-closed before building auth context—fix: reject JWT critical headers #231 by rejecting JWTs with unsupported crit headers, and the main PR by rejecting system_admin/platform_admin role claims on the legacy fallback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A guard stands at the HMAC gate,
Checking roles before it's late,
Platform admins now must flee,
OIDC paths are where they be! ✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Reject platform admin HMAC sessions' accurately describes the main change: rejecting system_admin and platform_admin claims on the legacy HMAC fallback session path.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hmac-system-admin-session-boundary-20260529

Comment @coderabbitai help to get the list of available commands and usage tips.

@seonghobae
seonghobae merged commit 80716de into master May 29, 2026
16 checks passed
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