From b2c7578653fbec75201a5fb3485d113a1b1466f2 Mon Sep 17 00:00:00 2001 From: Rui Xue Date: Tue, 14 Jul 2026 00:34:54 +0000 Subject: [PATCH] fix(sessions): guard all auto-mint paths + strict config/body parsing (gate review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gate review follow-ups for the #6022 no-leak rule: every non-deliberate newSession/session-new call site passes worktree:false; only deliberate New Chat inherits the config default. - Strict boolean config read: only YAML true enables the default; quoted strings/ints/lists/dicts/null fall to the safe no-worktree default instead of truthiness-coercing. - Presence-based explicit detection: 'worktree' in body — an explicit worktree:null is an opt-out, never a fall-through to config. - Guarded remaining auto-mint call sites with explicit worktree:false: profile switch (panels.js), promptWorkspacePath + switchToWorkspace blank-page binds (panels.js), promptNewFile + promptNewFolder (ui.js), terminal auto-session (commands.js). - Tests: worktree:null vs config-true (factory must not be called), strict-boolean malformed-shape matrix, static regression gates for every auto-mint site + a bare-body sweep so future auto-mint paths can't silently omit the key; updated the cmdTerminal preflight assertion for the new call shape. --- api/routes.py | 12 ++++- static/commands.js | 4 +- static/panels.js | 10 ++-- static/ui.js | 8 +++- tests/test_embedded_workspace_terminal.py | 4 +- .../test_issue6022_worktree_config_default.py | 40 ++++++++++++++++ tests/test_issue6022_worktree_ui_static.py | 48 +++++++++++++++++++ 7 files changed, 117 insertions(+), 9 deletions(-) diff --git a/api/routes.py b/api/routes.py index 72f6a921..e10dfaed 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7596,7 +7596,11 @@ def _worktree_default_from_config(profile: str | None) -> bool: cfg_dict = get_config_for_profile_home(get_hermes_home_for_profile(profile)) else: cfg_dict = get_config_for_profile_home(None) - return bool((cfg_dict or {}).get("worktree", False)) + # Strict boolean: only a real YAML `true` opts in. Any other shape + # ("true", 1, [], {}, null, ...) is malformed for this key and must + # fall to the safe no-worktree default rather than truthiness-coerce + # into minting worktrees. + return (cfg_dict or {}).get("worktree", False) is True except Exception: # Config resolution must never break session creation. logger.warning("failed to read worktree config default", exc_info=True) @@ -13914,7 +13918,11 @@ def handle_post(handler, parsed) -> bool: # worktree (e.g. the boot-time auto-bind) send ``worktree: false`` # explicitly. raw_worktree = body.get("worktree") - worktree_explicit = raw_worktree is not None + # Presence-based, not truthiness-based: a client that sends the key at + # all (even ``worktree: null``) has spoken explicitly and never falls + # through to the config default. ``null`` parses as non-true below, + # i.e. an explicit opt-out — only a genuinely ABSENT key inherits. + worktree_explicit = "worktree" in body if worktree_explicit: worktree_requested = ( raw_worktree is True diff --git a/static/commands.js b/static/commands.js index 18728773..dc6a0d43 100644 --- a/static/commands.js +++ b/static/commands.js @@ -801,7 +801,9 @@ async function cmdTerminal(){ const first=(data&&data.workspaces||[])[0]; S._profileSwitchWorkspace=(data&&data.last)||(first&&first.path)||null; } - await newSession(); + // System-minted session (#6022): opening the terminal auto-creates a + // session — explicit worktree:false so the config default can't leak. + await newSession(false, {worktree: false}); if(typeof renderSessionList==='function') await renderSessionList(); } if(!S.session||!S.session.workspace){ diff --git a/static/panels.js b/static/panels.js index 2576e1d0..2af98dd1 100644 --- a/static/panels.js +++ b/static/panels.js @@ -6303,7 +6303,9 @@ async function promptWorkspacePath(){ const ws=(typeof S._profileDefaultWorkspace==='string'&&S._profileDefaultWorkspace)||''; if(!ws)return; try{ - const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); + // System-minted session (#6022): worktree:false is explicit so a config + // worktree default can't leak a worktree from a workspace prompt. + const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws,worktree:false})}); if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];if(typeof syncTopbar==='function')syncTopbar();if(typeof renderMessages==='function')renderMessages();if(typeof renderSessionList==='function')await renderSessionList();} }catch(e){showToast(t('workspace_switch_failed')+e.message);return;} if(!S.session)return; @@ -6339,7 +6341,9 @@ async function switchToWorkspace(path,name){ const ws=path||(typeof S._profileDefaultWorkspace==='string'&&S._profileDefaultWorkspace)||''; if(!ws){showToast(t('no_workspace'));return;} try{ - const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); + // System-minted session (#6022): explicit worktree:false — a workspace + // switch from a blank page is not deliberate New Chat intent. + const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws,worktree:false})}); if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];if(typeof syncTopbar==='function')syncTopbar();if(typeof renderMessages==='function')renderMessages();if(typeof renderSessionList==='function')await renderSessionList();} }catch(e){if(typeof setStatus==='function')setStatus(t('switch_failed')+e.message);return;} if(!S.session)return; @@ -7129,7 +7133,7 @@ async function switchToProfile(name) { // The current session has messages and belongs to the previous profile. // Start a new session for the new profile so nothing gets cross-tagged. const workspaceVisible = typeof _workspacePanelMode !== 'undefined' && _workspacePanelMode !== 'closed'; - await newSession(false, {awaitWorkspaceLoad: workspaceVisible}); + await newSession(false, {awaitWorkspaceLoad: workspaceVisible, worktree: false}); if (_switchGen !== _profileSwitchGeneration) return false; // Keep topbar chips (workspace/profile) in sync after creating the // new profile-scoped session. diff --git a/static/ui.js b/static/ui.js index 4b3f96ca..60c63997 100644 --- a/static/ui.js +++ b/static/ui.js @@ -19566,7 +19566,9 @@ async function promptNewFile(targetDir = S.currentDir || '.'){ const ws=(typeof S._profileDefaultWorkspace==='string'&&S._profileDefaultWorkspace)||''; if(!ws) return; try{ - const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); + // System-minted session (#6022): explicit worktree:false — creating a + // file from a blank page must not inherit the config worktree default. + const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws,worktree:false})}); if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];syncTopbar();renderMessages();await renderSessionList();} }catch(e){setStatus(t('create_failed')+e.message);return;} } @@ -19597,7 +19599,9 @@ async function promptNewFolder(targetDir = S.currentDir || '.'){ const ws=(typeof S._profileDefaultWorkspace==='string'&&S._profileDefaultWorkspace)||''; if(!ws) return; try{ - const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); + // System-minted session (#6022): explicit worktree:false — creating a + // folder from a blank page must not inherit the config worktree default. + const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws,worktree:false})}); if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];syncTopbar();renderMessages();await renderSessionList();} }catch(e){setStatus(t('folder_create_failed')+e.message);return;} } diff --git a/tests/test_embedded_workspace_terminal.py b/tests/test_embedded_workspace_terminal.py index c816008d..2f4a6fe6 100644 --- a/tests/test_embedded_workspace_terminal.py +++ b/tests/test_embedded_workspace_terminal.py @@ -170,7 +170,9 @@ def test_terminal_slash_command_preflights_remote_backend_before_session_create( assert "syncTerminalBackendState(data)" in cmd_block assert "data&&data.terminal_remote_backend" in cmd_block assert "_terminalRemoteBackendUnsupportedMessage" in cmd_block - assert cmd_block.index("data&&data.terminal_remote_backend") < cmd_block.index("await newSession()") + # #6022: the auto-mint passes explicit worktree:false so the config + # default can't leak a worktree from opening the terminal. + assert cmd_block.index("data&&data.terminal_remote_backend") < cmd_block.index("await newSession(false, {worktree: false})") def test_terminal_v1_does_not_expose_send_to_chat_action(): diff --git a/tests/test_issue6022_worktree_config_default.py b/tests/test_issue6022_worktree_config_default.py index 1006737c..bc409edf 100644 --- a/tests/test_issue6022_worktree_config_default.py +++ b/tests/test_issue6022_worktree_config_default.py @@ -142,6 +142,27 @@ def test_explicit_false_beats_config_default_true(tmp_path, monkeypatch): assert session.get("worktree_path") in (None, "") +def test_explicit_null_beats_config_default_true(tmp_path, monkeypatch): + """Sending the key at all — even ``worktree: null`` — is an explicit + statement and must never fall through to the config default.""" + repo, worktree = _mk_repo_dirs(tmp_path) + + def _fail(workspace): + pytest.fail("worktree must not be created when body sends worktree=null") + + monkeypatch.setattr(worktrees, "create_worktree_for_workspace", _fail) + captured = _post_session_new( + tmp_path, + monkeypatch, + {"workspace": str(repo), "profile": "default", "worktree": None}, + config_default=True, + workspace_dir=repo, + ) + assert captured["status"] == 200 + session = captured["payload"]["session"] + assert session.get("worktree_path") in (None, "") + + def test_explicit_true_with_config_default_off_creates_worktree(tmp_path, monkeypatch): repo, worktree = _mk_repo_dirs(tmp_path) captured = _post_session_new( @@ -224,6 +245,25 @@ def test_worktree_default_resolves_named_profile_home(monkeypatch, tmp_path): assert seen["home"] == tmp_path / "work" +def test_worktree_default_requires_strict_boolean_true(monkeypatch): + """Only a real YAML ``true`` opts in — malformed shapes (quoted strings, + ints, lists, dicts, null) must fall to the safe no-worktree default, not + truthiness-coerce into minting worktrees.""" + for malformed in ("true", "yes", 1, [True], {"enabled": True}, None, 0, ""): + monkeypatch.setattr( + routes, + "get_config_for_profile_home", + lambda home, _v=malformed: {"worktree": _v}, + ) + assert routes._worktree_default_from_config(None) is False, ( + f"non-boolean config value {malformed!r} must not enable worktrees" + ) + monkeypatch.setattr( + routes, "get_config_for_profile_home", lambda home: {"worktree": True} + ) + assert routes._worktree_default_from_config(None) is True + + def test_worktree_default_is_fail_soft_on_config_errors(monkeypatch): def boom(home): raise RuntimeError("config unavailable") diff --git a/tests/test_issue6022_worktree_ui_static.py b/tests/test_issue6022_worktree_ui_static.py index e57b2f81..fe16f943 100644 --- a/tests/test_issue6022_worktree_ui_static.py +++ b/tests/test_issue6022_worktree_ui_static.py @@ -42,6 +42,54 @@ def test_onboarding_session_sends_explicit_worktree_false(): assert "worktree: false" in finish +def test_profile_switch_session_sends_explicit_worktree_false(): + src = read("static/panels.js") + assert ( + "await newSession(false, {awaitWorkspaceLoad: workspaceVisible, worktree: false});" + in src + ) + + +def test_workspace_bind_prompts_send_explicit_worktree_false(): + # promptWorkspacePath + switchToWorkspace both auto-mint a session from a + # blank page; each must opt out of the config default explicitly. + src = read("static/panels.js") + assert ( + src.count( + "body:JSON.stringify({workspace:ws,worktree:false})" + ) + >= 2 + ), "panels.js blank-page session mints must send worktree:false" + # No panels.js session/new call may omit the worktree key. + assert "body:JSON.stringify({workspace:ws})" not in src + + +def test_file_and_folder_creation_send_explicit_worktree_false(): + src = read("static/ui.js") + for fn in ("async function promptNewFile", "async function promptNewFolder"): + block = src[src.index(fn) :] + block = block[: block.index("\n}\n")] + assert "worktree:false" in block, f"{fn} must opt out of the config default" + assert "body:JSON.stringify({workspace:ws})" not in src + + +def test_terminal_auto_session_sends_explicit_worktree_false(): + src = read("static/commands.js") + assert "await newSession(false, {worktree: false});" in src + + +def test_no_bare_session_new_posts_remain_in_static_js(): + # Belt-and-suspenders: no static file may POST /api/session/new with a + # body that has a workspace but silently omits the worktree key on an + # auto-mint path. All known auto-mint sites are asserted above; this + # catches future regressions of the same shape. + for name in ("panels.js", "ui.js"): + src = read(f"static/{name}") + assert "body:JSON.stringify({workspace:ws})" not in src, ( + f"static/{name}: auto-mint session/new must pass explicit worktree:false" + ) + + def test_deliberate_new_chat_paths_do_not_pin_worktree(): # Sidebar "New Chat" and command paths must NOT pass an explicit worktree # value — they inherit the server-side config default by design.