Repository navigation
[Feature]: Shared, hardened local-directory install primitive for extensions, presets, workflows, and step packages #4793
Copy link
Copy link
Open
Labels
triage-can-waitVerdict: valid and in-scope but deprioritized; held behind the evidence gateVerdict: valid and in-scope but deprioritized; held behind the evidence gate
Description
Problem Statement
Four component types can be installed from a local directory (--dev, or as the second half of an archive install): extensions, presets, workflow packages, and custom step packages (#4695 / #4769). Each implements its own traversal, validation, and copy, and they have drifted apart in safety guarantees:
| Path | Copy | Entry/byte/depth limits | .git / __pycache__ |
Symlinks | Project lock |
|---|---|---|---|---|---|
Extension --dev (ExtensionManager.install_from_directory) |
shutil.copytree(..., ignore=.extensionignore) |
none | copied unless listed in .extensionignore |
not checked; followed | none |
Preset --dev (PresetManager.install_from_directory) |
bare shutil.copytree |
none | copied | not checked; followed | none |
Workflow package (_validate_local_workflow_package + _install_workflow_package) |
os.walk validation, then shutil.copytree |
none | walked and copied | rejected at validation; not rechecked during copy | .workflow-install.lock |
Step package (workflows/step/installer.py, #4769) |
streamed os.scandir + chunked O_NOFOLLOW copy |
512 entries / 50 MiB / depth 32 | pruned (matches bundles/packager.py) |
rejected in validation and copy | .step-install.lock |
Consequences:
- Unbounded work and disk use: an extension or preset developed in a git checkout copies its whole
.gitinto the project, and none of the extension, preset, or workflow paths bound the size of the source tree. The archive paths are bounded bysafe_extract_archive(MAX_ZIP_ENTRIES = 512, 50 MiB), but only beforeinstall_from_directoryruns. Local sources skip those limits entirely. - Symlink escape:
shutil.copytreewith the defaultsymlinks=Falsecopies whatever a symlink points to. A local extension or preset can therefore pull files from outside its directory into the project. The workflow path checks symlinks first, butcopytreedoes not re-check them, so a symlink swapped in after validation is still followed. - Inconsistent exclusions: there are three different exclusion policies.
bundles/packager.pyand the step installer prune.git/__pycache__/.DS_Store, extensions rely on a per-package.extensionignore, and presets and workflows exclude nothing. - Inconsistent concurrency: workflow and step installs use the shared
_exclusive_project_lockintroduced in feat(workflows): install custom step types from local dirs and archives #4769, but extension and preset installs and removals are not serialized. - Duplicated maintenance: each hardening fix lands in one installer only. Review on feat(workflows): install custom step types from local dirs and archives #4757 and feat(workflows): install custom step types from local dirs and archives #4769 repeatedly raised findings that apply to all four paths. They were deferred there, as agreed in feat(workflows): install custom step types from local dirs and archives #4769 (comment).
Proposed Solution
Extract one CLI-independent primitive, for example specify_cli/_local_install.py, and move all four installers onto it:
- Bounded traversal: stream
os.scandir, prune a shared exclusion set, never follow symlinks, and enforce a configurable entry/byte/depth budget, checking it before each directory listing is sorted. Each component supplies its own limits; the defaults match the existing archive limits. - Safe copy: a copy that doesn't follow symlinks and re-checks each entry as it goes (
O_NOFOLLOW, device/inode check after open, chunked byte budget). Where the platform allows, use fd-relative operations (dir_fd) so a source swapped mid-copy cannot redirect it. This covers the deferred "fd-relative local-install primitive" item from feat(workflows): install custom step types from local dirs and archives #4769. - Staging and atomic commit: stage in a same-filesystem temp directory, validate the staged tree, then commit with a rename, rolling back on failure. The step installer already works this way.
- Shared project lock: extension and preset install/remove/update use
_exclusive_project_lock, each with its own lock file, as workflows and steps already do. - Exclusion policy: keep the default exclusion set and make
.extensionignorean addition to it rather than the only mechanism. The open question is whether presets and workflows should get an equivalent ignore file.
Proposed delivery, one PR each:
- Introduce the primitive, with tests, by extracting it from the step installer (no behaviour change for steps).
- Move presets onto it (smallest surface).
- Move extensions onto it, keeping
.extensionignoresemantics. - Move workflow packages onto it.
- Add the shared lock to extensions and presets.
Alternatives Considered
- Patch each installer separately: the fastest path for any single finding, and the reason the paths drifted apart in the first place. Each future hardening round would have to be repeated four times.
- Route local directories through the archive pipeline (zip the source, then
safe_extract_archive): reuses the existing limits, but still walks the full source tree to build the archive, and doubles I/O on every--deviteration. - Limits only, no safe copy: closes the unbounded-work gap but leaves symlink escape through
copytree.
Component
Specify CLI (initialization, commands)
Use Cases
- A developer iterating on an extension or preset inside its own git checkout runs
specify extension add --dev .and gets only the package contents, not.git. - A CI job installing components from a checkout cannot pull in files from outside the source directory through a symlink.
- A bundle install that installs several component types from local sources gets the same limits, symlink policy, and locking for each.
- Maintainers apply one hardening fix to every install path at once.
Acceptance Criteria
- One shared module owns traversal, limits, exclusions, safe copy, staging, and commit. Extension, preset, workflow, and step installers call it and no longer contain their own
copytree/os.walkcopy logic. - Local-directory installs of all four component types:
- reject any symlink or special file in the retained tree,
- enforce documented entry/byte/depth limits, and stop reading once over budget,
- skip
.git,__pycache__, and.DS_Storewithout entering them.
- Extension
.extensionignorebehaviour is preserved. - Extension and preset install/remove/update run under a project lock. Race tests cover install versus remove in both orders.
- Every behaviour change has a positive and a negative test, and each fixed gap has a regression test that fails before the change.
docs/referencefor extensions, presets, and workflows states the limits and exclusions.
Additional Context
- Deferred items from feat(workflows): install custom step types from local dirs and archives #4769 ("Deferred" section of the PR body): the symlink race during recursive copy, and extending the shared project lock to extensions and presets.
- Maintainer direction: feat(workflows): install custom step types from local dirs and archives #4769 (comment) ("first land this ... then yes a refactoring along those lines is appreciated after we scope it properly").
- Reference implementation:
src/specify_cli/workflows/step/installer.py(feat(workflows): install custom step types from local dirs and archives #4769) andsrc/specify_cli/bundles/packager.py_collect_files. - Related: [Feature]: Add local (--dev) installation for custom workflow step types #4695, feat(workflows): install custom step types from local dirs and archives #4757.
AI Disclosure
Drafted by OpenCode (model: Claude Opus 5.5, autonomous) on behalf of @markuswondrak, based on a codebase survey of the four install paths; extent: research and issue text. Reviewed by @markuswondrak before filing.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
triage-can-waitVerdict: valid and in-scope but deprioritized; held behind the evidence gateVerdict: valid and in-scope but deprioritized; held behind the evidence gate
Activity