Activity

1. mnriem

   added

   needs-triage

   bug

   triage-nice-to-haveVerdict: evidence-backed fix or greenlit feature — land after reviewVerdict: evidence-backed fix or greenlit feature — land after review

   bug-assess

   and removed

   needs-triage

   bug

   on Sep 25, 2026
2. github-actions commented on Sep 25, 2026

   

   github-actionsbot

   on Sep 25, 2026 – with GitHub Actions

   Contributor

   More actions

   Bug assessment — harden-removal-paths: Valid · severity medium

   ---

   Bug Assessment: Harden filesystem removal of installed components

   • Slug: harden-removal-paths
   • Created: 2026-09-25T16:28:26Z
   • Source: issue [Bug]: Harden filesystem removal of installed components #4744
   • Verdict: valid
   • Severity: medium

   Report (summarized)

   Issue #4744 reports that extension and preset removal construct destructive paths directly from persisted registry IDs. It requests independent ownership/installed-state checks, validation that IDs are a single safe path component, rejection of symlink targets, and protection against symlink-redirection of extension config backups. The report is explicitly a source-level finding; no destructive live reproduction was performed. It also asks for an audit of integration uninstall paths while retaining existing guards for workflow and step removal.

   The issue has no additional comments. The issue body references an open remediation pull request, but this assessment is against the checked-out main source.

   Symptom

   ExtensionManager.remove() and PresetManager.remove() verify registry membership but then append the persisted ID to a filesystem root and perform backup, recursive deletion, or directory removal without a dedicated ID/target safety check at the destructive boundary. If an attacker or corrupted state can place an unsafe ID in the registry and a user explicitly invokes removal, deletion or backup writes may be redirected outside the intended component directory. Bundle removal delegates to these same managers.

   Reproduction

   1. In an isolated disposable project, add an unsafe extension ID and matching registry metadata, plus a corresponding filesystem entry or symlink target.
   2. Invoke extension removal and inspect the constructed extension and .backup paths; repeat with an unsafe preset ID and direct preset removal.
   3. Do not target real user files. The report does not provide a safe live reproduction, so the impact is established from source inspection rather than an executed destructive test.

   [NEEDS CLARIFICATION: The report does not specify the exact malformed IDs and registry/file-state fixture used by the reporter.]

   Suspected Code Paths

   • src/specify_cli/extensions/__init__.py:2950 — ExtensionManager.remove() only checks registry.is_installed(extension_id) before constructing extension_dir at line 2975.
   • src/specify_cli/extensions/__init__.py:3010 — config backup destination is built as .backup / extension_id; mkdir() and copy2() operate on that derived path before shutil.rmtree(extension_dir) at line 3023.
   • src/specify_cli/presets/_manager.py:631 — PresetManager.remove() checks registry membership at line 640, then constructs pack_dir from pack_id at line 671 and recursively removes it at line 861.
   • src/specify_cli/bundles/adapters.py:335 and src/specify_cli/bundles/primitives.py:234 — bundle removal delegates to the extension/preset managers, so recorded bundle component IDs reach the same destructive operations.
   • src/specify_cli/integrations/manifest.py:325 — integration uninstall has lexical containment and symlink handling for the final tracked path at lines 353–386, but the audit should confirm that tampered recorded paths cannot traverse through symlinked ancestors and that force behavior remains intentional.
   • src/specify_cli/bundles/references.py:23 — bundle resolution uses registry membership for presets and extensions, which is an ownership lookup but does not itself validate the IDs used later by removal.

   Root Cause Hypothesis

   The managers treat a registry key as both an ownership assertion and a trusted filesystem component. Registry membership prevents removal of an unknown component, but it does not establish that the key is a valid single directory name or that the resolved target is the expected non-symlink directory. Extension backup handling adds a second derived path whose destination is not independently checked. Confidence: high.

   Proposed Remediation

   Preferred: Add a shared, narrowly scoped component-ID validator for extension and preset removal that accepts only the repository’s canonical ID format and rejects empty values, path separators, ./.., and other multi-component or platform-special names. At the start of each manager’s destructive remove() operation, validate the ID, confirm the registry entry is a valid installed/owned record, and validate the derived target without following symlinks. Require an existing target to be a real directory (or preserve the documented missing-target behavior); fail explicitly on invalid or symlinked targets. Apply equivalent no-follow validation to the extension backup root and backup destination before creating directories or copying config files. Keep valid removal, missing-target, and keep_config behavior unchanged.

   For integration uninstall, retain manifest hash ownership checks and final symlink refusal, but audit and, if needed, validate every ancestor of a recorded path without following symlinks before unlinking. Do not ban symlinked project or storage parents globally; scope the check to the recorded removal path.

   Alternatives:

   • Resolve the target and compare it to the expected component root before deletion. This is simpler for normal directories but must separately handle missing targets and can be less explicit than rejecting symlinks at each path component.
   • Migrate registries to normalized/sanitized IDs on write and reject legacy unsafe entries on read. This improves future state but requires migration/error handling and does not by itself protect already-corrupted registries during removal.

   Files likely to change:

   • src/specify_cli/extensions/__init__.py
   • src/specify_cli/presets/_manager.py
   • A shared path/ID validation helper near the component managers, if existing validation utilities do not fit
   • src/specify_cli/integrations/manifest.py if the ancestor audit identifies a gap
   • src/specify_cli/bundles/primitives.py or bundle removal tests only if delegation needs explicit error translation
   • tests/specify_cli/extensions/ and tests/specify_cli/presets/
   • tests/specify_cli/bundles/ and tests/integrations/test_manifest.py for the audit coverage

   Tests to add or update:

   • Invalid extension and preset IDs are rejected before any deletion or registry mutation.
   • Valid single-component IDs remove their own directories and registry entries.
   • Missing targets preserve existing successful removal semantics.
   • Symlinked extension/preset targets are rejected without deleting the symlink target.
   • Extension config backup paths cannot be redirected through a symlink; valid backups still preserve config files.
   • Bundle removal propagates a clear failure for an invalid or unsafe component ID.
   • Integration manifest uninstall covers recorded paths with symlinked ancestors, normal files, modified files, and force behavior.

   Risks & Considerations

   • Tightening ID validation may reject legacy registry entries; provide an explicit error and preserve the registry for repair rather than silently deleting it.
   • Symlink checks must use no-follow filesystem operations and must not accidentally remove a target outside the project.
   • keep_config, config backup preservation, missing-target handling, and post-removal reconciliation are compatibility-sensitive.
   • The issue requires an explicit removal command and coordinated persisted-state changes, so this is a conditional local filesystem-integrity issue rather than an automatic-trigger vulnerability; that limits blast radius and supports medium severity.

   Open Questions

   • [NEEDS CLARIFICATION: What exact ID grammar is canonical for all currently supported extension and preset IDs, including legacy IDs?]
   • [NEEDS CLARIFICATION: Should an existing symlinked component directory be reported as an error or as a non-removable/skipped target in the public CLI output?]
   • [NEEDS CLARIFICATION: Should --force ever permit removal of a symlink entry, or should symlinks remain unconditionally rejected for component managers?]

   Generated by 🐛 Assess Bug from Labeled Issue for #4744 · copilot · gpt52codex · 2.66 AIC · ⌖ 7.27 AIC · ⊞ 22.4K · ◷
3. github-actions

   added

   severity-medium

   on Sep 25, 2026