fix(kanban): containment + prevention for scratch-cleanup data loss (#28818)#31708
Merged
Conversation
…root (#28818) A board's ``default_workdir`` (e.g. ``hermes kanban boards set-default-workdir my-board /path/to/real/source``) is copied into ``tasks.workspace_path`` for tasks created without an explicit ``workspace_kind``. Those tasks default to ``workspace_kind='scratch'``, so completion calls ``_cleanup_workspace`` and unconditionally runs ``shutil.rmtree(wp, ignore_errors=True)`` — deleting the user's real source tree as if it were disposable scratch storage. Add ``_is_managed_scratch_path()`` and gate ``_cleanup_workspace`` on it: only delete paths under ``HERMES_KANBAN_WORKSPACES_ROOT`` (the worker-side override the dispatcher injects) or under the active kanban home's ``kanban/`` subtree (covering both the legacy default-board root and per-board ``kanban/boards/<slug>/workspaces`` roots). Anything else gets a warning log and is left alone, so a misconfigured ``default_workdir`` can no longer destroy user data on task completion.
Copilot review on PR #28819 flagged that `_is_managed_scratch_path` accepted the entire `<kanban_home>/kanban` subtree as managed scratch storage. With that, a task whose `workspace_kind='scratch'` and `workspace_path` was mis-set to `<kanban_home>/kanban`, `.../kanban/logs`, or a board's metadata directory (e.g. `.../kanban/boards/<slug>` without the `workspaces/` child) would pass the containment guard and let task completion `shutil.rmtree` Hermes' own DB, metadata, and log subtrees. Tighten the guard: * Allowed roots are now exclusively `workspaces/` directories — the `HERMES_KANBAN_WORKSPACES_ROOT` override, `<kanban_home>/kanban/workspaces`, and each `<kanban_home>/kanban/boards/<slug>/workspaces` discovered on disk. * Require strict descendancy: a path equal to a root itself is rejected too, because deleting a workspaces root would wipe every task's scratch dir at once. Add a regression test covering the three Copilot-named attack paths (kanban root, kanban/logs, board root without `workspaces/`) plus the workspaces-root-itself case, and confirm the inner task-id dir still matches.
…8818) Board defaults represent persistent project checkouts. Scratch workspaces are auto-deleted on completion and must stay under the per-board scratch root that resolve_workspace() creates. Inheriting default_workdir for a scratch task pointed the cleanup path at the user's source tree — the data-loss vector documented in #28818. The containment guard in _cleanup_workspace (just added) is the safety rail. This commit prevents the bad state from being created in the first place: only persistent kinds (dir/worktree) inherit board defaults. Tests updated to cover the new semantics: scratch with default_workdir set keeps workspace_path=None; dir/worktree still inherits the board default. Salvaged from PR #31315 by @leeseoki0 — prevention layer on top of the #28819 containment fix by @briandevans. Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
This was referenced May 24, 2026
Contributor
🔎 Lint report:
|
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvage of #28819 by @briandevans + #31315 by @leeseoki0, addressing P0 issue #28818 (Kanban can delete a real user source directory on task completion).
Summary
Two-layer fix for the data-loss vector. (1) _cleanup_workspace now refuses to shutil.rmtree anything outside Hermes-managed scratch roots. (2) create_task no longer inherits board default_workdir for kind=scratch, so the bad state stops being created in the first place.
Threat
A board's default_workdir is set to a real source tree. A task is created without an explicit workspace_kind, so it defaults to scratch. Task completion calls _cleanup_workspace which sees kind='scratch' + workspace_path= and shutil.rmtree's the user's project. Worker sibling tasks then crash with FileNotFoundError on os.getcwd().
Changes
Containment (fix(kanban): refuse to rmtree workspace_path outside managed scratch root (#28818) #28819, @briandevans): new _is_managed_scratch_path() whitelisting
Strict descendancy required — the roots themselves are NOT managed (would wipe all tasks' scratch dirs at once), and sibling subtrees like /logs or board metadata are explicitly rejected.
Prevention (Guard Kanban scratch workspace cleanup #31315, @leeseoki0): create_task now only inherits default_workdir when workspace_kind in {'dir', 'worktree'}. Scratch tasks keep workspace_path=None, and resolve_workspace() creates the per-board scratch dir on demand.
Test plan
Combined tests cover both layers:
Salvage scope
Co-authored-by: briandevans 252620095+briandevans@users.noreply.github.com
Co-authored-by: leeseoki0 leeseoki@makestar.com
Closes #28818
Closes #28819
Closes #31315