From decb1037bc44c076a4e701a81a149e0022e7a664 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Mon, 22 Jun 2026 00:18:39 -0400 Subject: [PATCH] Keep escape grants browser-confirmed and refreshable (#4582) --- api/routes.py | 4 + static/workspace.js | 21 ++-- ...t_issue4582_workspace_escape_navigation.py | 102 +++++++++++++++++- tests/test_symlink_cycle_detection.py | 7 +- 4 files changed, 119 insertions(+), 15 deletions(-) diff --git a/api/routes.py b/api/routes.py index 1d45071d..2b931382 100644 --- a/api/routes.py +++ b/api/routes.py @@ -12949,6 +12949,10 @@ def _read_json_request_body(handler, *, max_bytes: int = 4096) -> dict: def _handle_escape_authorize(handler, parsed, body: dict | None = None): if handler.command != "POST": return bad(handler, "method not allowed", 405) + if not _is_browser_unsafe_request(handler): + return bad(handler, "browser confirmation required", 403) + if not _check_csrf(handler): + return bad(handler, _csrf_rejection_error(handler), 403) if body is None: try: body = _read_json_request_body(handler) diff --git a/static/workspace.js b/static/workspace.js index 1063740f..dd83a860 100644 --- a/static/workspace.js +++ b/static/workspace.js @@ -252,16 +252,17 @@ async function authorizeWorkspaceEscapeNavigation(item){ if(!S.session || !item || !item.path) return null; const normalizedPath = _normalizeWorkspaceRelPath(item.path); const exactGrant = _workspaceEscapeExactGrant(normalizedPath); - if(exactGrant) return exactGrant; - const ok = await showConfirmDialog({ - title: item.name || normalizedPath, - message: t('external_link_open_confirm'), - confirmLabel: t('dialog_confirm_btn'), - danger: false, - hideCancel: true, - focusCancel: false, - }); - if(!ok) return null; + if(!exactGrant){ + const ok = await showConfirmDialog({ + title: item.name || normalizedPath, + message: t('external_link_open_confirm'), + confirmLabel: t('dialog_confirm_btn'), + danger: false, + hideCancel: true, + focusCancel: false, + }); + if(!ok) return null; + } try{ const data = await api('/api/escape/authorize', { method: 'POST', diff --git a/tests/test_issue4582_workspace_escape_navigation.py b/tests/test_issue4582_workspace_escape_navigation.py index a2dc241a..0096493f 100644 --- a/tests/test_issue4582_workspace_escape_navigation.py +++ b/tests/test_issue4582_workspace_escape_navigation.py @@ -7,6 +7,7 @@ import shutil import subprocess import urllib.error import urllib.request +import urllib.parse import pytest @@ -30,11 +31,16 @@ def _get_bytes(path: str) -> bytes: return response.read() -def _post_json(path: str, body: dict | None = None) -> tuple[dict, int]: +def _browser_headers() -> dict[str, str]: + parsed = urllib.parse.urlparse(BASE) + return {"Origin": f"{parsed.scheme}://{parsed.netloc}"} + + +def _post_json(path: str, body: dict | None = None, headers: dict[str, str] | None = None) -> tuple[dict, int]: req = urllib.request.Request( BASE + path, data=json.dumps(body or {}).encode(), - headers={"Content-Type": "application/json"}, + headers={"Content-Type": "application/json", **(headers or {})}, ) try: with urllib.request.urlopen(req, timeout=10) as response: @@ -89,9 +95,16 @@ class TestIssue4582EscapeNavigationLive: escape_row = {entry["name"]: entry for entry in root_listing["entries"]}["escape-file.txt"] assert escape_row["target_outside_workspace"] is True + denied, denied_status = _post_json( + "/api/escape/authorize", + {"session_id": sid, "path": "escape-file.txt"}, + ) + assert denied_status == 403, denied + auth, status = _post_json( "/api/escape/authorize", {"session_id": sid, "path": "escape-file.txt"}, + headers=_browser_headers(), ) assert status == 200, auth assert auth["path"] == "escape-file.txt" @@ -131,7 +144,14 @@ class TestIssue4582EscapeNavigationLive: escape_row = {entry["name"]: entry for entry in root_listing["entries"]}["escape"] assert escape_row["target_outside_workspace"] is True - auth, status = _post_json("/api/escape/authorize", {"session_id": sid, "path": "escape"}) + denied, denied_status = _post_json("/api/escape/authorize", {"session_id": sid, "path": "escape"}) + assert denied_status == 403, denied + + auth, status = _post_json( + "/api/escape/authorize", + {"session_id": sid, "path": "escape"}, + headers=_browser_headers(), + ) assert status == 200, auth assert auth["path"] == "escape" assert auth["is_dir"] is True @@ -173,12 +193,17 @@ class TestIssue4582EscapeNavigationLive: (workspace / "escape").symlink_to(outside) sid = _make_session(workspace) - auth, status = _post_json("/api/escape/authorize", {"session_id": sid, "path": "escape"}) + auth, status = _post_json( + "/api/escape/authorize", + {"session_id": sid, "path": "escape"}, + headers=_browser_headers(), + ) assert status == 200, auth nested_auth, nested_status = _post_json( "/api/escape/authorize", {"session_id": sid, "path": "escape/nested-escape"}, + headers=_browser_headers(), ) assert nested_status in (403, 404), nested_auth @@ -285,6 +310,75 @@ const apiFns = runner(S, showConfirmDialog, api, showToast, t, URLSearchParams); ] assert result["toasts"][0][0] == "external_link_read_only" + def test_exact_grant_click_reauthorizes_without_reprompt(self): + helper_block = _workspace_escape_helper_block() + js = ( + "const helperBlock = " + + json.dumps(helper_block) + + ";\n" + + r""" +const S = { + session: { session_id: 'sess-1' }, + currentDir: '.', + _escapeGrants: { + escape: { + sessionId: 'sess-1', + path: 'escape', + token: 'tok-old', + expiresAt: Date.now() + 60_000, + isDir: true, + }, + }, +}; +const confirmCalls = []; +const apiCalls = []; +const toasts = []; +const showConfirmDialog = async (opts) => { confirmCalls.push(opts); return true; }; +const api = async (path, opts) => { + apiCalls.push({ path, method: opts && opts.method, body: opts && opts.body }); + return { + token: 'tok-new', + path: 'escape', + expires_at: 4102444800, + is_dir: true, + read_only: true, + }; +}; +const showToast = (...args) => { toasts.push(args); }; +const t = (key) => key; +const runner = new Function( + 'S', 'showConfirmDialog', 'api', 'showToast', 't', 'URLSearchParams', + helperBlock + '; return { authorizeWorkspaceEscapeNavigation, _workspaceEscapeExactGrant };' +); +const apiFns = runner(S, showConfirmDialog, api, showToast, t, URLSearchParams); +(async () => { + const grant = await apiFns.authorizeWorkspaceEscapeNavigation({ path: 'escape', name: 'escape' }); + console.log(JSON.stringify({ + grant, + stored: apiFns._workspaceEscapeExactGrant('escape'), + confirmCalls, + apiCalls, + toasts, + })); +})().catch((err) => { + console.error(err && err.stack ? err.stack : String(err)); + process.exit(1); +}); +""" + ) + result = _run_node(js) + assert result["grant"]["token"] == "tok-new" + assert result["stored"]["token"] == "tok-new" + assert result["confirmCalls"] == [] + assert result["apiCalls"] == [ + { + "path": "/api/escape/authorize", + "method": "POST", + "body": "{\"session_id\":\"sess-1\",\"path\":\"escape\"}", + } + ] + assert result["toasts"][0][0] == "external_link_read_only" + def test_external_rows_authorize_then_open(self): src = UI_JS.read_text(encoding="utf-8") assert "authorizeWorkspaceEscapeNavigation(item)" in src diff --git a/tests/test_symlink_cycle_detection.py b/tests/test_symlink_cycle_detection.py index e1682cef..e3442742 100644 --- a/tests/test_symlink_cycle_detection.py +++ b/tests/test_symlink_cycle_detection.py @@ -15,6 +15,7 @@ recursion. Covers: import json import os import pathlib +import urllib.parse import urllib.request import urllib.error import tempfile @@ -33,8 +34,12 @@ def get(path): def post(path, body=None): url = BASE + path data = json.dumps(body or {}).encode() + headers = {"Content-Type": "application/json"} + if path == "/api/escape/authorize": + parsed = urllib.parse.urlparse(BASE) + headers["Origin"] = f"{parsed.scheme}://{parsed.netloc}" req = urllib.request.Request(url, data=data, - headers={"Content-Type": "application/json"}) + headers=headers) try: with urllib.request.urlopen(req, timeout=10) as r: return json.loads(r.read()), r.status