fix(sessions): guard all auto-mint paths + strict config/body parsing (gate review)
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.
This commit is contained in:
parent
5f638a1694
commit
b2c7578653
7 changed files with 117 additions and 9 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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){
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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;}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue