Skip to content

fix(security): validate X-User-Id against trusted owner whitelist to prevent BOLA#2023

Open
namann5 wants to merge 3 commits into
utksh1:mainfrom
namann5:fix/bola-x-user-id-header-auth-bypass
Open

fix(security): validate X-User-Id against trusted owner whitelist to prevent BOLA#2023
namann5 wants to merge 3 commits into
utksh1:mainfrom
namann5:fix/bola-x-user-id-header-auth-bypass

Conversation

@namann5

@namann5 namann5 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a Critical (CVSS ~9.8) Broken Object-Level Authorization (BOLA) vulnerability where the X-User-Id header — which scopes all data access (vault secrets, tasks, findings, reports) — was accepted without verification. Any client possessing the shared API key could exfiltrate every other workspace's data by spoofing this header.

Fixes: #2021

Changes

Root Cause

backend/secuscan/auth.py:239-245resolve_owner_id() trusted the X-User-Id header at face value with no server-side validation. This header flows into every data query as owner_id, meaning an attacker could read any workspace's vault secrets (decrypted to plaintext), tasks, findings, and reports.

Fix

  1. backend/secuscan/config.py — Added trusted_owner_ids: List[str] = [] configuration option
  2. backend/secuscan/auth.pyresolve_owner_id() now validates X-User-Id against the whitelist:
    • Empty whitelist (default): header is ignored, all requests scoped to DEFAULT_OWNER_ID
    • Populated whitelist: only listed owner IDs accepted; others fall back to default with a logged warning
  3. Tests updated — Unit tests mock the settings properly; integration tests use conftest.py fixture with trusted_owner_ids=["alice", "bob"]

Security Impact

Before After
Any X-User-Id value accepted Only whitelisted values accepted
Default: all values work Default: header ignored entirely
Silent cross-workspace access Logged warning on rejection

Configuration

# Single-user (default) — header ignored
# No configuration needed

# Multi-user — set allowed workspace IDs
SECUSCAN_TRUSTED_OWNER_IDS=team-alpha,team-beta

Testing

  • 20/20 unit tests pass for auth module
  • Integration test fixture updated with trusted_owner_ids=["alice", "bob"]
  • No merge conflicts with main

…prevent BOLA

The X-User-Id header that scopes all data access (vault secrets, tasks,
findings, reports) was accepted without verification, allowing any caller
with the shared API key to read another workspace's data by spoofing the
header.

This fix adds a configurable trusted_owner_ids whitelist:
- Empty (default): X-User-Id is ignored, all requests use DEFAULT_OWNER_ID
- Populated: only listed owner IDs are accepted; others fall back to default

Resolves: utksh1#2021

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1039bc24df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/secuscan/config.py
@utksh1 utksh1 added level:critical 80 pts difficulty label for critical or high-impact PRs type:security Security work category bonus label type:bug Bug fix work category bonus label area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests labels Jul 14, 2026

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A whitelist does not fix this BOLA: any client holding the shared API key can still send X-User-Id: team-alpha and impersonate that listed owner. Please bind owner identity to an authenticated principal or a cryptographically verified trusted-proxy assertion; do not accept a client-controlled ownership header as authorization. Add a regression test covering an attacker who sends another allowlisted ID.

namann5 added 2 commits July 15, 2026 13:59
…ontrolled whitelist

Replace the trusted_owner_ids whitelist approach with trusted-proxy
validation.  The whitelist was bypassable because any client holding
the shared API key could spoof X-User-Id: team-alpha.

Now resolve_owner_id() only accepts X-User-Id when request.client.host
is in settings.trusted_proxies.  This binds owner identity to an
authenticated infrastructure component (reverse proxy / SSO layer)
rather than trusting a client-controlled header.

Changes:
- auth.py: Add _is_trusted_proxy(), validate source before accepting header
- test_auth_owner_resolution.py: Add proxy-based tests + attacker regression test
- test_auth_helpers.py: Update tests for trusted-proxy model
- conftest.py: Add 'testclient' to trusted_proxies for integration tests
@namann5

namann5 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@utksh1
Replaced the flawed trusted_owner_ids whitelist with trusted-proxy IP validation. Now resolve_owner_id() only accepts X-User-Id when request.client.host is in settings.trusted_proxies. This binds owner identity to the infrastructure component (reverse proxy), not a client-controlled header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests level:critical 80 pts difficulty label for critical or high-impact PRs type:bug Bug fix work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants