Skip to content

fix(security): owner-gate /api/configure-pat (hotfix)#42

Merged
dgokeeffe merged 1 commit into
mainfrom
fix/configure-pat-owner-gate
May 17, 2026
Merged

fix(security): owner-gate /api/configure-pat (hotfix)#42
dgokeeffe merged 1 commit into
mainfrom
fix/configure-pat-owner-gate

Conversation

@dgokeeffe
Copy link
Copy Markdown
Collaborator

@dgokeeffe dgokeeffe commented May 17, 2026

Summary

Hotfix for a PAT-impersonation vector discovered in pen testing 2026-05-17. The /api/configure-pat endpoint is intentionally exempt from the before_request auth gate (the owner needs to reach it before the app has any credentials), but without an in-handler owner check, any workspace-SSO'd user could submit their own valid PAT and persistently impersonate the owner.

The vulnerability

app.py:951 (pre-fix) — no idempotency or owner check. The endpoint:

  1. Validates the submitted token via SCIM (passes — it's a real PAT)
  2. Sets os.environ['DATABRICKS_TOKEN'] = attacker_pat
  3. Mints new short-lived tokens via the rotator → all under attacker's identity
  4. Revokes the owner's previous PAT as a side-effect of revoke_bootstrap_token()
  5. Rewrites every CLI's credential file with attacker's PAT

From that point, every Claude/Codex/Gemini/OpenCode/Hermes/databricks CLI call made by the owner is audit-logged under the attacker's identity. The attacker can review their own PAT's audit history to reconstruct everything the owner did.

Exploitable at any time the app is reachable — not bootstrap-window only.

The fix

One in-handler guard at the top of configure_pat():

if _is_databricks_apps() and app_owner:
    if get_request_user() != app_owner:
        return jsonify({"error": "Forbidden"}), 403

Reuses existing helpers (_is_databricks_apps, get_request_user, app_owner) — no new auth machinery introduced. Short-circuits to "allow" only when app_owner is unresolved at startup, matching the rest of the auth surface's bootstrap-window behaviour. Without this carve-out, no owner could ever finish bootstrap.

Verification

Unit (locked-in regression): 4 new tests in tests/test_auth_enforcement.py::TestConfigurePatAuth:

  • test_configure_pat_denied_for_non_owner → 403
  • test_configure_pat_allowed_for_owner → 400 (Token required — past gate)
  • test_configure_pat_bootstrap_window_allowed → not-403 when app_owner=None
  • test_configure_pat_case_insensitive_for_owner → mixed-case header accepted

All pass. 231/231 total unit tests pass.

E2E (against deployed daveok): custom one-off probe (.humantokens/configure_pat_e2e_probe.py, not committed):

  • [1/3] Owner POST empty body → HTTP 400 `Token required` ✓ (owner gate is open)
  • [2/3] Forged `X-Forwarded-Email` + bogus PAT → HTTP 400 `Invalid token` ✓ (proves the Databricks Apps proxy strips client-supplied headers; identity is cookie-derived)
  • [3/3] No-cookies POST → HTTP 401 ✓ (proxy enforces SSO)

Why this is being merged as a hotfix

Sathish, requesting post-hoc review — flag anything you'd want refactored, will roll into the next normal PR.

Test plan

  • Unit tests pass (231/231)
  • E2E probe against live deployment passes (3/3)
  • Hotfix-deployed daveok confirms behaviour through the real proxy stack
  • Sathish post-hoc review

This pull request and its description were written by Isaac.

@dgokeeffe
fix(security): owner-gate /api/configure-pat against PAT impersonation
457829f 
/api/configure-pat is exempt from the before_request auth gate
(intentionally — needed before the owner has set up the app). Without
an in-handler owner check, any workspace-SSO'd user who reaches the
app could submit their own valid PAT and persistently impersonate the
owner — every subsequent CLI/AI-agent call would run under the
submitter's identity, and `revoke_bootstrap_token()` would even take
out the legitimate owner's previous PAT as a side-effect.

This adds a guard at the top of `configure_pat()`:
- if running on Databricks Apps AND `app_owner` is resolved
- and the request's user differs from `app_owner`
- return 403 Forbidden

`app_owner` is set in `initialize_app()` before gunicorn binds the
port, so it's reliably populated by request time. The guard
short-circuits to "allow" only when owner resolution failed at startup
(matching the rest of the auth surface's bootstrap-window behaviour) —
without this carve-out, no owner could ever finish bootstrap.

Verification:
- 4 new unit tests in test_auth_enforcement.py::TestConfigurePatAuth
  covering: denied-for-non-owner, allowed-for-owner, bootstrap-window
  allowed, case-insensitive owner match. All pass.
- 231/231 total unit tests pass.
- E2E probe against deployed daveok (.humantokens/configure_pat_e2e_probe.py):
  * owner POST empty body → 400 "Token required" ✓
  * forged X-Forwarded-Email + bogus PAT → 400 "Invalid token" ✓
    (proves the Databricks Apps proxy is authoritative on identity —
     forged headers are stripped at the edge)
  * no-cookies POST → 401 Unauthorized ✓

Discovered during white-box pen test 2026-05-17 (see
.humantokens/2026-05-17-pentest-findings.md, P2 NEW finding).

Co-authored-by: Isaac
@dgokeeffe dgokeeffe merged commit dba47f3 into main May 17, 2026
This was referenced May 17, 2026
Open
Merged
Merged
dgokeeffe added a commit that referenced this pull request May 18, 2026
@dgokeeffe
fix(security): idempotent /api/configure-pat — defence-in-depth (#51)
ae6d7ad 
PR #42 closed the impersonation vector by gating /api/configure-pat
on the owner. That's the primary control: only the owner can submit
a PAT.

But the endpoint is still non-idempotent. Once a PAT is active, the
owner (or anything driving the owner's browser — XSS, malicious tab,
session-hijacking malware, or just an accidental double-submit) can
POST a different PAT and the rotator will swap to it, revoking the
previously-active token as a side effect.

This adds an idempotency guard immediately after the owner check:

  if pat_rotator.token and not pat_rotator.is_token_expired:
      return 409 "PAT already configured. Restart the app to reconfigure."

Bootstrap is single-shot by design — the rotator handles all subsequent
credential refreshes automatically. There's no legitimate reason for
the frontend to call configure-pat twice in one app lifetime (confirmed
by grep — the only caller is the PAT-bootstrap path inside the
"PAT not valid" branch of createPane()).

The expired-token escape hatch is preserved: if the rotator times out
because all sessions were reaped (long idle), is_token_expired returns
True and the owner can re-bootstrap without an app restart.

409 Conflict (not 403 Forbidden) is the right status here: the issue
isn't authorization (the owner DOES have permission) — it's that the
operation conflicts with current state.

Tests:
- test_configure_pat_rejected_when_already_configured: rotator alive →
  even owner gets 409. Mocks pat_rotator.token + is_token_expired as
  PropertyMocks on the class (instance properties don't accept patching
  through mock.patch.object directly).
- test_configure_pat_allowed_when_token_expired: rotator timed out →
  owner can still bootstrap (gets 400 "Token required" with empty body,
  proving they passed both gates).
- All 6 TestConfigurePatAuth cases pass; 233/233 total.

Co-authored-by: Isaac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@datasciencemonkey datasciencemonkey Awaiting requested review from datasciencemonkey

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dgokeeffe