[Feature]: Shared, hardened local-directory install primitive for extensions, presets, workflows, and step packages

Offen
  • Stimmt etwas nicht?
#4,793 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 1 Tag

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Anfängerfreundlichkeit
35/100
Issue-Typ
Refactoring
Klarheit
Klar beschrieben
Aktivitätsstatus
Aktiv
Tech-Stack
python
Bereich
cli, security

Rechercherichtung

Start with src/specify_cli/workflows/step/installer.py and src/specify_cli/bundles/packager.py _collect_files as reference implementations, then inspect the four component installers named in the issue. The proposed delivery is staged across separate PRs, beginning with a shared primitive and tests extracted from the step installer. Done means all four installers use it, the listed safety, locking, and exclusion acceptance criteria pass, and the specified reference docs describe the limits and exclusions.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

triage-can-wait
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 .git into the project, and none of the extension, preset, or workflow paths bound the size of the source tree. The archive paths are bounded by safe_extract_archive (MAX_ZIP_ENTRIES = 512, 50 MiB), but only before install_from_directory runs. Local sources skip those limits entirely.
  • Symlink escape: shutil.copytree with the default symlinks=False copies 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, but copytree does not re-check them, so a symlink swapped in after validation is still followed.
  • Inconsistent exclusions: there are three different exclusion policies. bundles/packager.py and 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_lock introduced in #4769, but extension and preset installs and removals are not serialized.
  • Duplicated maintenance: each hardening fix lands in one installer only. Review on #4757 and #4769 repeatedly raised findings that apply to all four paths. They were deferred there, as agreed in https://github.com/github/spec-kit/pull/4769#issuecomment-5874451519.
Proposed Solution

Extract one CLI-independent primitive, for example specify_cli/_local_install.py, and move all four installers onto it:

  1. 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.
  2. 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 #4769.
  3. 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.
  4. 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.
  5. Exclusion policy: keep the default exclusion set and make .extensionignore an 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:

  1. Introduce the primitive, with tests, by extracting it from the step installer (no behaviour change for steps).
  2. Move presets onto it (smallest surface).
  3. Move extensions onto it, keeping .extensionignore semantics.
  4. Move workflow packages onto it.
  5. 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 --dev iteration.
  • 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.walk copy 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_Store without entering them.
  • Extension .extensionignore behaviour 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/reference for extensions, presets, and workflows states the limits and exclusions.
Additional Context
  • Deferred items from #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: https://github.com/github/spec-kit/pull/4769#issuecomment-5874451519 ("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 (#4769) and src/specify_cli/bundles/packager.py _collect_files.
  • Related: #4695, #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.

Vorherrschende Sprache
Python
Sterne
149k
Forks
13.4k
Ø Merge
1 T. 5 Std.
Gemergte PRs (30 T.)
223

Entwicklungsumgebung

In Codespaces öffnen

Startet den Dev-Container des Projekts im Browser, mit Ihrem eigenen GitHub-Konto.

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus github/spec-kit

Alle Issues in github/spec-kit

Ähnliche Issues

Weitere Issues zu Python

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.