mirror of
https://github.com/github/spec-kit.git
synced 2026-03-24 06:13:08 +00:00
fix: path traversal guard, rollback extension re-registration, lifecycle docs
- remove_tracked_files: validate resolved path stays within project_path before unlinking; reject entries with '../' that escape the project root - Rollback: call _reregister_extension_commands() during rollback (same as success path) so extension files are properly restored - AgentBootstrap: comprehensive lifecycle flow docstring documenting the setup → finalize_setup → get_tracked_files → check_modified → teardown chain and explaining why tracking all files is safe (hash check)
This commit is contained in:
@@ -2740,14 +2740,12 @@ def agent_switch(
|
||||
rollback_resolved = resolve_agent_pack(current_agent, project_path=project_path)
|
||||
rollback_bs = load_bootstrap(rollback_resolved.path, rollback_resolved.manifest)
|
||||
rollback_files = rollback_bs.setup(project_path, script_type, options)
|
||||
# Re-register extension commands (same as the success path)
|
||||
rollback_ext_files = _reregister_extension_commands(project_path, current_agent)
|
||||
rollback_bs.finalize_setup(
|
||||
project_path,
|
||||
agent_files=rollback_files,
|
||||
extension_files=list(
|
||||
(project_path / p).resolve()
|
||||
for p in old_tracked_ext
|
||||
if (project_path / p).is_file()
|
||||
),
|
||||
extension_files=rollback_ext_files,
|
||||
)
|
||||
console.print(f" [green]✓[/green] {current_agent} restored")
|
||||
except Exception:
|
||||
|
||||
@@ -200,6 +200,24 @@ class AgentBootstrap:
|
||||
|
||||
Subclasses override :meth:`setup` and :meth:`teardown` to define
|
||||
agent-specific lifecycle operations.
|
||||
|
||||
**Lifecycle flow (setup → tracking → teardown):**
|
||||
|
||||
1. ``setup()`` installs files and returns **every** file it created
|
||||
(agent commands, shared scripts, templates, etc.).
|
||||
2. The CLI calls ``finalize_setup(agent_files, extension_files)``
|
||||
which SHA-256 hashes each file and writes the manifest at
|
||||
``.specify/agent-manifest-<id>.json``.
|
||||
3. During switch/remove, the CLI reads the manifest via
|
||||
``get_tracked_files()`` and calls ``check_modified_files()``
|
||||
to detect changes. Modified files are listed and the user
|
||||
is prompted for confirmation.
|
||||
4. ``teardown()`` delegates to ``remove_tracked_files()`` which
|
||||
**compares hashes before deleting** — only files whose
|
||||
SHA-256 still matches the original are removed. Modified
|
||||
files are preserved unless ``--force`` is used. This makes
|
||||
it safe to track all files (including shared project
|
||||
infrastructure) without risk of deleting user work.
|
||||
"""
|
||||
|
||||
def __init__(self, manifest: AgentManifest):
|
||||
@@ -637,8 +655,16 @@ def remove_tracked_files(
|
||||
)
|
||||
|
||||
removed: List[str] = []
|
||||
project_root = project_path.resolve()
|
||||
for rel_path, original_hash in entries.items():
|
||||
abs_path = project_path / rel_path
|
||||
# Guard against path traversal: reject entries that resolve
|
||||
# outside the project directory (e.g. via "../" in a
|
||||
# tampered manifest).
|
||||
try:
|
||||
abs_path.resolve().relative_to(project_root)
|
||||
except ValueError:
|
||||
continue
|
||||
if abs_path.is_file():
|
||||
if original_hash and _sha256(abs_path) != original_hash:
|
||||
# File was modified since installation — skip unless forced
|
||||
|
||||
Reference in New Issue
Block a user