From 457829ffc833f4f38932b2ca63e7d4d1ac5bf0fa Mon Sep 17 00:00:00 2001 From: David O'Keeffe Date: Sun, 17 May 2026 19:39:10 +1000 Subject: [PATCH] fix(security): owner-gate /api/configure-pat against PAT impersonation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /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 --- app.py | 12 +++++ tests/test_auth_enforcement.py | 99 ++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/app.py b/app.py index 0c63cad..ff2ae55 100644 --- a/app.py +++ b/app.py @@ -951,6 +951,18 @@ def pat_status(): @app.route("/api/configure-pat", methods=["POST"]) def configure_pat(): """Accept a user-provided PAT, validate it, and start rotation.""" + # Hotfix: only the resolved owner may (re-)configure the PAT. Without this, + # any workspace-SSO'd user who reaches the app can submit their own valid + # PAT and persistently impersonate the owner — every CLI call would then + # run under the submitter's identity. app_owner is set in initialize_app() + # before gunicorn binds, so it's reliably populated by request time on + # Databricks Apps; this guard short-circuits to "allow" only when owner + # resolution failed (matches the rest of the auth surface's behaviour). + if _is_databricks_apps() and app_owner: + if get_request_user() != app_owner: + logger.warning(f"Rejected configure-pat from non-owner {get_request_user()} (owner: {app_owner})") + return jsonify({"error": "Forbidden"}), 403 + data = request.json token = data.get("token", "").strip() if not token: diff --git a/tests/test_auth_enforcement.py b/tests/test_auth_enforcement.py index 578daa1..42030c3 100644 --- a/tests/test_auth_enforcement.py +++ b/tests/test_auth_enforcement.py @@ -114,6 +114,105 @@ def test_resize_allowed_for_owner(self): self._assert_not_denied("POST", "/api/resize", {"session_id": "fake", "cols": 80, "rows": 24}) +# --------------------------------------------------------------------------- +# 1b. /api/configure-pat MUST enforce owner check (hotfix) +# --------------------------------------------------------------------------- + +class TestConfigurePatAuth: + """The PAT bootstrap endpoint is auth-exempt at the before_request gate + (intentionally — needed before the owner has set up). It MUST still gate + on owner once app_owner is resolved, otherwise any workspace-SSO'd user + can submit their own PAT and persistently impersonate the owner. + """ + + def test_configure_pat_denied_for_non_owner(self): + app_module = _get_app_module() + original_owner = app_module.app_owner + try: + app_module.app_owner = "owner@databricks.com" + client = _make_client(app_module) + with mock.patch.object(app_module, "_is_databricks_apps", return_value=True): + resp = client.post( + "/api/configure-pat", + json={"token": "dapi-attacker"}, + headers={"X-Forwarded-Email": "intruder@evil.com"}, + ) + assert resp.status_code == 403, ( + f"POST /api/configure-pat should return 403 for non-owner, got {resp.status_code}" + ) + finally: + app_module.app_owner = original_owner + + def test_configure_pat_allowed_for_owner(self): + """Owner can still bootstrap. We don't run the rotator, just confirm + the auth guard doesn't return 403 — actual token validation is mocked + out separately and not in scope here.""" + app_module = _get_app_module() + original_owner = app_module.app_owner + try: + app_module.app_owner = "owner@databricks.com" + client = _make_client(app_module) + with mock.patch.object(app_module, "_is_databricks_apps", return_value=True): + # Don't pass a token — we expect 400 ("Token required"), which + # proves the request got past the owner check. + resp = client.post( + "/api/configure-pat", + json={}, + headers={"X-Forwarded-Email": "owner@databricks.com"}, + ) + assert resp.status_code != 403, ( + f"POST /api/configure-pat should not return 403 for owner, got {resp.status_code}" + ) + assert resp.status_code == 400, ( + f"Owner with empty body should get 400 'Token required', got {resp.status_code}" + ) + finally: + app_module.app_owner = original_owner + + def test_configure_pat_bootstrap_window_allowed(self): + """During the brief window where app_owner hasn't been resolved yet + (e.g., Apps API hiccup at startup), the endpoint must still accept + the request — otherwise the owner can never finish bootstrap. + This is intentional and documented in the in-handler comment.""" + app_module = _get_app_module() + original_owner = app_module.app_owner + try: + app_module.app_owner = None # unresolved + client = _make_client(app_module) + with mock.patch.object(app_module, "_is_databricks_apps", return_value=True): + resp = client.post( + "/api/configure-pat", + json={}, + headers={"X-Forwarded-Email": "anyone@databricks.com"}, + ) + # Should NOT be 403 — should fall through to "Token required" (400) + assert resp.status_code != 403, ( + f"During bootstrap (app_owner unresolved), configure-pat should not 403, got {resp.status_code}" + ) + finally: + app_module.app_owner = original_owner + + def test_configure_pat_case_insensitive_for_owner(self): + """Owner email casing from the SSO header must match the lowercased + app_owner — same case-insensitive contract as the rest of auth.""" + app_module = _get_app_module() + original_owner = app_module.app_owner + try: + app_module.app_owner = "owner@databricks.com" + client = _make_client(app_module) + with mock.patch.object(app_module, "_is_databricks_apps", return_value=True): + resp = client.post( + "/api/configure-pat", + json={}, + headers={"X-Forwarded-Email": "Owner@Databricks.COM"}, + ) + assert resp.status_code != 403, ( + f"Mixed-case owner header should be accepted, got {resp.status_code}" + ) + finally: + app_module.app_owner = original_owner + + # --------------------------------------------------------------------------- # 2. Case-insensitive email matching # ---------------------------------------------------------------------------