* fix(integrations): wrap a non-UTF-8 catalog response
`_fetch_single_catalog` decodes the response body with `.decode("utf-8")`
before handing it to `json.loads`. A non-UTF-8 body therefore raises
`UnicodeDecodeError`, which is a sibling of `json.JSONDecodeError` under
`ValueError` rather than a subclass of it, so neither the `URLError` nor the
`JSONDecodeError` handler catches it.
The raw exception escapes `_get_merged_integrations`, whose
`except IntegrationCatalogError` is specifically designed to warn and skip a
bad catalog and carry on with the remaining ones. One catalog served over a
misconfigured proxy or truncated mid-multibyte-sequence thus takes down
`specify integration search` entirely instead of degrading to a warning.
Wrap it in `IntegrationCatalogError`, matching the convention already used
for the same decode in `authentication/azure_devops.py`, which lists
`UnicodeDecodeError` alongside `JSONDecodeError`.
Note that the cache-read path in this same method already tolerates this via
its `UnicodeError` clause; only the network path was unguarded.
Two regression tests: one pins the wrapped-error contract on the fetch, and
one covers the behaviour that actually motivates it — a broken catalog is
skipped with a warning while a healthy sibling catalog still resolves.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(integrations): use the shared urlopen routing fixture
The raw-bytes helper patched `open_url` wholesale, which skipped the real
URL validation and redirect handling inside it. This module already imports
`route_opener_open_through_urlopen`, the repo's shared fixture that routes
`build_opener().open()` back through `urlopen` for exactly this reason, so
patching `urlopen` instead keeps the stub effective while still exercising
`open_url` itself.
Renamed to `_patch_urlopen_bytes` to sit alongside the existing
`_patch_urlopen`, whose signature it now mirrors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(integrations): restore the non-UTF-8 handler
The previous commit reverted the source change by accident while reworking
the tests, leaving the regression tests passing against an unfixed module.
Restores the `except UnicodeDecodeError` clause.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(events): skip an unreadable command template
_render_command_template() read the resolved template with a bare
read_text(), so a template file that exists but cannot be read or
decoded (permission error, non-UTF-8 bytes) crashed event dispatch with
a raw OSError/UnicodeDecodeError. Every sibling failure in this path
(missing template, unresolvable command) already returns None so the
dispatcher falls back cleanly.
Wrap the read and return None on OSError/UnicodeDecodeError, matching
the sibling contract.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: cover the OSError half of the unreadable-template boundary
Review follow-up: add a mocked PermissionError case so both promised
exception paths are protected under privileged CI.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`_local_manifest_source` handles three local bundle sources. The directory
and `bundle.yml` branches both go through `BundleManifest.from_file` ->
`load_yaml`, which converts a parse failure into a `BundlerError`. The `.zip`
branch instead parses inline with a bare `_yaml.safe_load`.
`yaml.YAMLError` derives directly from `Exception` -- it is neither a
`ValueError` nor an `OSError` -- so it escapes `bundle_install`'s
`except BundlerError` and reaches the user as a raw
`yaml.parser.ParserError` traceback.
The remote counterpart of this same call, `_download_manifest`, already
guards it and even names `_yaml.YAMLError` explicitly. Only the local zip
path was missed, so the same corrupt manifest is reported cleanly when
fetched from a catalog but crashes when installed from disk.
Before, for the identical malformed bundle.yml:
specify bundle install ./bundle-dir -> Error: Invalid YAML in ... (exit 1)
specify bundle install ./bundle.yml -> Error: Invalid YAML in ... (exit 1)
specify bundle install ./bundle.zip -> ParserError traceback
Two regression tests: one pins the `BundlerError` contract on the zip
branch, and one drives all three local sources through the CLI to assert
they now fail alike.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(agent-context): recurse for nested plans in Python mtime fallback
The Python port's mtime fallback discovered plans with a one-level
specs/*/plan.md glob, so a scoped layout created via
SPECIFY_FEATURE_DIRECTORY (specs/<scope>/<feature>/plan.md) was missed when
feature.json is absent — the fallback returned no plan and the managed
context section omitted the 'at <plan>' line. The bash and PowerShell twins
were already fixed to recurse (#3024); the Python twin was left behind.
Switch to specs.rglob('plan.md') with the same symlink-safe containment check
the bash twin uses (resolve each candidate and confirm it stays within the
project root before ranking by mtime), so a plan reached through a specs/
symlink pointing outside the project is not selected.
Adds parity regression tests (vs bash and vs PowerShell) covering a nested
specs/<scope>/<feature>/plan.md; both fail on the pre-fix one-level glob.
Fixes#3733
* test(agent-context): cover symlink containment in the mtime fallback
The recursive fallback resolves each candidate before the relative_to()
containment check, but nothing exercised that path. Add a parity test for a
plan reachable only through a specs/ symlink pointing outside the project:
relative_to() is lexical and would accept it, emitting an in-project-looking
path for an out-of-project file. Both the bash twin and the Python port skip
it, so the "at <plan>" line is omitted.
Also correct the module docstring, which still described the fallback as
scanning specs/*/plan.md one level deep.
* fix(init): escape user-supplied values in `specify init` output
commands/init.py interpolated the project name, --integration/--script values
and paths straight into Rich markup f-strings. It was the only CLI command
module without escaping -- extensions, presets, workflows and integrations all
wrap user-controlled display values already.
Two consequences, both reproduced end-to-end through the real CLI:
1. SILENT WRONG OUTPUT. `specify init "proj [v2]"` exits 0 and creates the
directory, but the Next Steps panel prints
1. Go to the project folder: cd proj
Rich ate `[v2]` as a style tag, so the command the user copy-pastes fails.
2. CRASH AFTER SUCCESS. `specify init "app[/red]x"` creates the project and
then dies with MarkupError("closing tag '[/red]' ... doesn't match any open
tag") -> exit 1 with a traceback for work that actually completed.
Wrap the user-controlled display values in rich.markup.escape: project name
(error/warning/conflict/next-steps), project and working paths, the echoed
--integration and --script values, and the agent folder in the gitignore hint.
Display only -- no control flow, exit codes or messages change, and escape is a
no-op for any value without a tag-shaped bracket run.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(init): shell-quote the project name in the Next Steps cd line
Rich-escaping stopped the brackets being swallowed, but the printed
command was still unusable for any name containing whitespace: `cd proj
v2` is two arguments in every shell.
$ cd proj v2 -> /bin/bash: line 1: cd: too many arguments (rc=1)
$ cd "proj v2" -> rc=0, lands in "proj v2"
Quote it for the host the same way _version._render_argv renders its
copy-pasteable installer command: subprocess.list2cmdline on Windows,
shlex.quote elsewhere. Windows must use double quotes -- cd 'my project'
is a path-not-found in cmd.exe, while cd "my project" is accepted by
cmd.exe, PowerShell and Git Bash alike. Names needing no quoting are
returned unchanged, so the common case is byte-identical.
Shell-quote inner, Rich-escape outer.
Tests execute the printed command through a real shell rather than only
inspecting the string, and pin that an ordinary name stays unquoted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(init): drop the now-redundant local escape imports that shadowed the module one
Rebasing onto main brought in three new extension-install helpers, and two
of them carry a function-local
from rich.markup import escape as _escape_markup
inside `register > init`. This PR adds the same import at module level, so
the locals made `_escape_markup` a local variable for the whole `init`
function — every use *before* those import lines then raised
UnboundLocalError: cannot access local variable '_escape_markup'
where it is not associated with a value
which broke `specify init` outright (7 of 8 tests in this file failed after
the rebase, all with exit_code 1).
The locals are redundant now that the module-level import exists, so remove
them. Verified with an AST scope walk that the only remaining
`_escape_markup` imports are the module-level one and the one inside
`_confirm_extension_url_trust`, which has no module-level use to shadow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(agent-context): recurse for nested plans in Python mtime fallback
The Python port's mtime fallback discovered plans with a one-level
specs/*/plan.md glob, so a scoped layout created via
SPECIFY_FEATURE_DIRECTORY (specs/<scope>/<feature>/plan.md) was missed when
feature.json is absent — the fallback returned no plan and the managed
context section omitted the 'at <plan>' line. The bash and PowerShell twins
were already fixed to recurse (#3024); the Python twin was left behind.
Switch to specs.rglob('plan.md') with the same symlink-safe containment check
the bash twin uses (resolve each candidate and confirm it stays within the
project root before ranking by mtime), so a plan reached through a specs/
symlink pointing outside the project is not selected.
Adds parity regression tests (vs bash and vs PowerShell) covering a nested
specs/<scope>/<feature>/plan.md; both fail on the pre-fix one-level glob.
Fixes#3733
* test(agent-context): cover symlink containment in the mtime fallback
The recursive fallback resolves each candidate before the relative_to()
containment check, but nothing exercised that path. Add a parity test for a
plan reachable only through a specs/ symlink pointing outside the project:
relative_to() is lexical and would accept it, emitting an in-project-looking
path for an out-of-project file. Both the bash twin and the Python port skip
it, so the "at <plan>" line is omitted.
Also correct the module docstring, which still described the fallback as
scanning specs/*/plan.md one level deep.
* fix: use missing_ok for temp file cleanup to avoid masking errors
`workflow status <run_id>` and `workflow resume <run_id>` both call
`RunState.load()`, and a prior fix aligned them on the FileNotFoundError
and ValueError boundaries. `resume` also handles OSError; `status` never
gained that handler.
So an unreadable `state.json` -- wrong permissions, an I/O error, or a
directory sitting where the file belongs -- escapes as a raw traceback
with no output at all, while `resume` on the same run prints a clean
`Error:` line and exits 1. `state_path.exists()` is True for a directory,
so the existing guard passes and `open()` raises.
Add the missing `except OSError` next to its siblings, using the same
`_escape_markup` + `typer.Exit(1)` shape, and routing through `err` so
the message lands on stderr under `--json` and the stdout JSON stream
stays parseable.
Two regression tests: the end-to-end CLI path (a directory in place of
state.json) and the `--json` stderr-routing path. Both fail without the
source change.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(init): scaffold managed .specify/.gitignore
Write a manifest-tracked `.specify/.gitignore` during shared-infra install
so machine-local Spec Kit state stays out of version control while everything
else under `.specify/` remains shareable:
- `feature.json` — the current-feature pointer, rewritten on every feature
switch (per-checkout state, not something to share).
- `extensions/*/local-config.yml` — per-machine extension config overrides.
The file is routed through the same overwrite/skip/preserve policy as shared
templates: `--force` refreshes it, user edits are preserved on re-init, and
uninstall removes it via the manifest. Addresses github/spec-kit#2304.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9
* docs: correct .specify/.gitignore uninstall claim
The file is tracked in the shared-infra manifest (speckit.manifest.json),
not the per-integration manifest that `specify integration uninstall` loads.
Shared infrastructure is deliberately preserved on uninstall
(see test_uninstall_preserves_shared_infra), so `.specify/.gitignore` is
left in place rather than removed. Reword the code comment and core.md note
to state the actual behavior; keep the true benefits (force-refresh and
preserve-on-edit).
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9
* revert: drop manual CHANGELOG.md edit
CHANGELOG.md is auto-generated; do not hand-edit it.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9
* test: add .specify/.gitignore to integration file inventories
The complete-file-inventory tests assert an exact match of every file
produced by `specify init`. Now that shared infra scaffolds a managed
`.specify/.gitignore`, add it to the expected inventories so the exact-match
assertions pass on both sh and ps script types.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9
* fix(scripts): stop check-prerequisites text mode crashing on a legacy code page
_check_file/_check_dir hard-code U+2713/U+2717 and print() them to
sys.stdout. On Windows sys.stdout falls back to the ANSI code page whenever
stdout is not a console — which is every time an agent or a workflow step
captures the output — and U+2713 is unencodable in cp1252:
stdout encoding: cp1252
UnicodeEncodeError: 'charmap' codec can't encode character '✓'
So text mode aborted right after printing "AVAILABLE_DOCS:", losing every
per-document line.
Fall back to ASCII when stdout cannot encode the glyph. "[OK]"/"[FAIL]" is
the rendering these markers already have in-tree: Test-FileExists in
scripts/powershell/common.ps1 emits exactly those, and
normalize_status_text in tests/parity_helpers.py maps the glyphs onto them,
so the twins already treat the two forms as equivalent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(scripts): cover both status markers in the cp1252 regression
Review catch: the fixture left every reported document absent (the empty
contracts/ also reports missing), so the test only ever called
_status_marker(False). The assertion was `"[OK]" in out or "[FAIL]" in out`,
which "[FAIL]" alone satisfied.
Proved the hole by mutation: replacing the fallback body with a bare
`return "[FAIL]"` — deleting the success branch outright — left the test
GREEN.
Add research.md so one document is present, and assert both markers
explicitly. The strengthened test now kills all three mutations:
fallback always "[FAIL]" -> FAILS (was passing)
fallback always "[OK]" -> FAILS
no fallback at all -> FAILS (the original bug)
unmutated -> 12 passed, 8 skipped
Missing documents are still present in the fixture, so the failure path
stays covered too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scripts): restore the _status_marker ASCII fallback
The previous commit on this branch unintentionally reverted the source fix
while adding the strengthened test, so the branch carried the test without
the implementation it tests.
Cause: my local verification script reverted the file for its red run with
`git checkout upstream/main -- <file>`, which writes the INDEX as well as
the working tree. Restoring the working-tree copy afterwards left main's
version staged, and the next commit captured it.
Restores the fix from 275663b. Verified: 12 passed / 8 skipped, and the red
run (source reverted) produces 1 new-vs-baseline failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(presets): return None for an unreadable layer in resolve_content
PresetResolver.resolve_content() reads the winning layer (and each
composition layer) with a bare read_text(), so a layer file that cannot
be read or decoded crashed command registration with a raw
OSError/UnicodeDecodeError. The docstring already promises 'Composed
content string, or None if not found', and since #3896
collect_all_layers() deliberately tolerates a non-UTF-8 legacy layer —
moving the crash here, where both callers (_register_commands and
_reconcile_composed_commands) are unguarded.
Return None when the winning or base layer cannot be read, treating an
unreadable layer like a missing one per the documented contract.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: cover the base guard and composing-layer read
Review follow-up: add an unreadable replace base beneath a valid
composing layer, and a mocked-PermissionError composing layer over a
valid base, so every new boundary and both exception types are covered.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ExtensionRegistry._load() catches json.JSONDecodeError and
FileNotFoundError to start fresh on a corrupted or missing registry, but
a .registry file with invalid UTF-8 bytes raised UnicodeDecodeError from
the text-mode read before JSON parsing began. Because the registry is
loaded in __init__, that bare traceback broke every extension command --
`specify extension list` on such a project exits with a raw
UnicodeDecodeError instead of the module's clean path.
Catch UnicodeDecodeError in the same clause: undecodable bytes are the
same corruption class as unparseable JSON, only the exception type
differs. OSError stays uncaught on purpose -- the data may be intact on
disk, and starting fresh would let a later _save() wipe it.
This is the exact twin of the PresetRegistry._load() fix in #3955; the
two registries are parallel implementations and only the preset side was
corrected. _get_installed_sibling_ids() already worked around this gap
locally by catching UnicodeError at its own call site; its comment is
updated to reflect that _load() now handles the case itself, with the
local catch kept as belt-and-braces against regression.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the remediation from the bug assessment on issue #3990.
After integration setup() and manifest.save(), when --force is used
(re-initializing an existing project), call _register_presets_for_agent
and _register_extensions_for_agent so that previously-installed presets
and extensions are recomposed on top of the freshly-regenerated core
files. Without this, preset-composed files reverted to pure core while
the preset registry continued to report them as installed.
This mirrors the same pattern already present in integration_upgrade()
(added in PR #3853 / issue #3849 for the upgrade path).
Refs #3990
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hermes overrides SkillsIntegration.setup() with its own copy of the
frontmatter parse and body strip, and Kimi's _is_speckit_generated_skill()
parses frontmatter independently, so all three carried the same
split("---", 2) bug the base class just fixed. A description such as
"Separate sections with --- markers" truncates the parsed frontmatter at the
embedded marker, dropping later keys and spilling the remainder into the
body; for Kimi that means a Speckit-generated skill is no longer recognized
on teardown and gets left behind.
Scan for a closing "---" on its own line instead. The body slice keeps
whatever trails the marker so output stays byte-for-byte identical for
well-formed templates.
* fix(archives): wrap the bare EOFError a truncated tar.gz raises
`tarfile` wraps most decompression failures in `TarError`, but a gzip
stream that ends before its end-of-stream marker escapes as a bare
`EOFError` from the gzip layer. `EOFError` derives from neither
`TarError` nor `OSError`, so it bypassed all three of the tar handlers
added with tar archive support (#3874):
- the format probe in `detect_archive_format`, which caught only
`tarfile.TarError`;
- `tarfile.open` in `safe_extract_tar`;
- member iteration in `safe_extract_tar`.
A truncated `.tar.gz` — an interrupted download, a partially written
file — therefore raised a raw `EOFError` straight through the caller's
`error_type`, so callers catching `ValueError`/`ExtensionError`/
`PresetError` never saw it. In `specify workflow add` the effect is worse
than a traceback: Typer treats a bare `EOFError` as a Ctrl-D abort, so
the command printed only "Aborted." with no diagnostic at all. The ZIP
twin reports "Invalid workflow archive: Invalid ZIP archive: <path>".
Route all three sites through a shared `_TAR_DECOMPRESSION_ERRORS`
tuple so they stay in sync. `zlib.error` is included alongside
`EOFError`: it is likewise neither a `TarError` nor an `OSError` and can
surface from a corrupt deflate block. `OSError` is kept only on the two
`safe_extract_tar` sites, which report genuine I/O failures; adding it
to the probe would silently swallow them instead.
Truncated tar.gz now reports the same clean, domain-typed error as the
ZIP path. Tests cover both the short prefix that fails in
`tarfile.open` and the longer ones that fail during member iteration —
`tarfile` decompresses lazily, so the leak surfaced at different sites
depending on how much of the stream survived.
Assisted-by: Claude Opus 5 (1M context)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(archives): cover the bare zlib.error a corrupt deflate block raises
Review feedback: the `zlib.error` arm of `_TAR_DECOMPRESSION_ERRORS` was
not exercised. Every regression added with the fix truncates a valid
deflate stream, which raises `EOFError`, so `zlib.error` could regress
independently of the EOF handling.
It is genuinely reachable, but only under a narrower condition than the
truncation cases. `tarfile` converts `zlib.error` to `ReadError` while
reading a member *header*, but the forward seek it performs to skip
member *data* (`tarfile.next`) sits outside that conversion, so a corrupt
region past the first header escapes raw. Reaching that seek needs
members larger than the gzip read buffer: with small members the whole
stream is decompressed during the first header read and the error is
wrapped. The new fixture therefore uses two 256 KiB members at
`compresslevel=1` — a ~7 KiB archive — corrupted past the midpoint so
the first header still reads clean.
Adds four tests: the two `safe_extract_tar` sites (plain and with a
caller-supplied `error_type`), the `safe_extract_archive` entry point
with a caller-supplied `error_type`, and a guard asserting the fixture
still reaches the module as a bare `zlib.error` — so if a future Python
wraps it, that fails loudly instead of the coverage silently decaying
into a duplicate of the `EOFError` cases.
Verified test-the-test: the three wrapping tests fail against the
unmodified `_download_security.py` with a raw
`zlib.error: Error -3 while decompressing data: invalid distance code`,
and pass with the fix.
Also corrects the scope claimed for the probe site. Fuzzing 2800 corrupt
archives never produced a bare `zlib.error` from `tarfile.open` alone,
because the only read it performs is the header read that `tarfile`
already converts. The probe's `zlib.error` arm is defensive, not
load-bearing; the tuple comment and a detection test now say so rather
than implying coverage that cannot exist.
Assisted-by: Claude Opus 5 (1M context)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(archives): make the corrupt-deflate fixture zlib-version independent
CI failure on macos-latest/3.13: `test_corrupt_deflate_fixture_raises_bare_zlib_error`
failed with `gzip.BadGzipFile: CRC check failed`. The other five pytest jobs
were fail-fast cancellations, not real failures, and ruff was already green.
The fixture built its corruption by XOR-ing 64 arbitrary bytes mid-stream.
Whether that produces a *structural* deflate error is zlib-version dependent:
on the macOS runner the mangled bytes still decoded, so the stream instead
failed the trailing gzip CRC check and raised `BadGzipFile` -- an `OSError`,
which the pre-fix `(TarError, OSError)` handler already caught. The guard test
exists precisely to catch that degradation, and it did its job.
Replaces the XOR with a deflate block header whose `BTYPE` is the reserved
value `0b11`. Every zlib rejects that identically as "invalid block type", and
it fails during decompression rather than at the CRC check, so no version can
turn it into a `TarError` or `OSError`. The stream is assembled by hand
(`compressobj(-15)` + explicit gzip header/trailer) so the invalid block lands
a controlled 256 KiB into the first member's data -- past the gzip read buffer,
so the first header still reads clean and the failure surfaces from the forward
seek in `tarfile.next`, which is the site the raw `zlib.error` escapes from.
A sweep over clean-prefix sizes confirms a wide margin: with 512 KiB members
every prefix from 160 KiB up yields a bare `zlib.error`, versus the transition
below ~131 KiB where `tarfile` still wraps it as `ReadError`. The hand-built
gzip header also zeroes the mtime field, so the fixture is now byte-identical
across builds instead of embedding a timestamp.
Strengthens the guard to assert what the fix actually depends on -- that the
exception is neither a `TarError` nor an `OSError` -- so the fixture cannot
silently decay into an already-caught type again.
Production code is unchanged from ef49acc; this is test-only.
Verified test-the-test by dropping the `zlib.error` arm from
`_TAR_DECOMPRESSION_ERRORS`: the three wrapping tests fail with the raw
`zlib.error: Error -3 while decompressing data: invalid block type`, and pass
with it restored. `tests/test_download_security.py`: 193 passed.
`ruff check src tests` (the exact CI command): all checks passed.
Assisted-by: Claude Opus 4.8 (1M context)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add regression tests for SkillsIntegration mixin that verify:
- Multiline (block-scalar) description round-trips byte-for-byte
- C0/DEL control characters in description survive YAML escaping
Tests properly isolate Path.home() for Hermes to prevent overwriting
a developer's real global skill directory.
Refs: #3392
_check_file/_check_dir hard-code U+2713/U+2717 and print() them to
sys.stdout. On Windows sys.stdout falls back to the ANSI code page whenever
stdout is not a console — which is every time an agent or a workflow step
captures the output — and U+2713 is unencodable in cp1252, so the document
listing aborted mid-report with UnicodeEncodeError.
This is the byte-identical twin of the block in
scripts/python/check_prerequisites.py, which I flagged in the PR for that
file rather than widening its scope.
Fall back to ASCII when stdout cannot encode the glyph. "[OK]"/"[FAIL]" is
the rendering these markers already have in-tree: Test-FileExists in
scripts/powershell/common.ps1 emits exactly those, and
normalize_status_text in tests/parity_helpers.py maps the glyphs onto them.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
`CommandRegistrar.render_frontmatter` calls `yaml.dump()` without `width=`,
so PyYAML applies its default ~80-column wrap and folds any long scalar onto
a continuation line.
A `description` longer than roughly 80 characters is therefore rendered as:
---
name: speckit-implement
description: Execute the implementation plan by processing and executing all tasks
defined in tasks.md
---
The YAML remains valid and round-trips faithfully through `yaml.safe_load`,
so this is not data loss. It is a shape inconsistency with real consequences:
- Hand-written core command templates always keep `description` on one line,
so preset- and extension-rendered commands do not match the files they sit
beside in the same directory.
- Consumers that read frontmatter line-wise rather than with a YAML parser
see the description truncated at the fold, followed by a stray line. Spec
Kit itself hand-builds SKILL.md frontmatter in the skills path (see #3391),
so this is not a hypothetical class of consumer.
- `speckit.implement`'s own description is 89 characters, so a preset that
overrides it hits this immediately.
`width=float("inf")` disables the line-wrapping only; escaping, quoting and
the handling of genuinely multi-line values are unchanged, since PyYAML
selects the scalar style before applying width.
Adds a regression test that fails without the change.
Verified against the repo's own suite: 6354 passed. Four failures in
tests/integrations/test_integration_subcommand.py are present on a clean
checkout too (ANSI escapes in captured output) and are unrelated.
* fix: skip corrupted run state files in list_runs
* fix: address review comments - add UnicodeDecodeError, dict validation, and regression tests
- Catch UnicodeDecodeError for invalid UTF-8 encoding
- Validate loaded JSON is a dict with required 'run_id' key
- Add 5 regression tests for corrupted state files
Fixes#3814
PresetRegistry._load() catches json.JSONDecodeError and
FileNotFoundError to start fresh on a corrupted or missing registry, but
a registry file with invalid UTF-8 bytes raised UnicodeDecodeError
before JSON parsing began, crashing every preset command.
Catch UnicodeDecodeError in the same clause: undecodable bytes are the
same corruption class as unparseable JSON. OSError stays uncaught on
purpose — the data may be intact on disk, and starting fresh would let a
later _save() wipe it (same fail-closed reasoning as the workflow
catalog cache loader).
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_merge_toml_fragment() and _remove_toml_entries() read the user's
config.toml with bare read_text() calls, so a non-UTF-8 (or otherwise
unreadable) file crashed install_integration_events() and
remove_integration_events() with a raw UnicodeDecodeError — and the
merge path regenerates the file from what it read, so it would have
discarded the user's bytes had it not crashed first. Every JSON
merge/remove path already goes through _load_user_json(), which skips
on an unreadable file to preserve user content (#22).
Abort the merge (returning False so the caller skips tracking, S5) and
skip the teardown cleanup with a warning, leaving the user's bytes
untouched in both directions.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The rescue-retry loop in install_from_directory() reads each staged
backup with bare stat()/read_bytes() calls, so a staged config that
cannot be read crashed the reinstall with a raw OSError. Every sibling
read in this path — the live twin four lines below, the packaged
baseline check, the mode sidecar — already catches OSError.
Treat an unreadable staged file like an uncomparable live config:
add it to the conflict set so both copies are preserved and the retry
aborts with the existing resolution guidance while dest_dir is still
untouched.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`requires.speckit_version` was presence-checked but never type-checked in
both the extension and preset manifest validators, so an unquoted YAML
`speckit_version: 1.0` (a float) passed validation and reached
`SpecifierSet(required)` in `check_compatibility()`.
That call is guarded by `except InvalidSpecifier` alone, which a
non-string escapes two different ways:
- a float/int/bool/None raises `TypeError: 'float' object is not
iterable` from the `SpecifierSet` constructor;
- a list or dict is an *iterable*, so `SpecifierSet` accepts it and the
failure surfaces much later as `AttributeError: 'str' object has no
attribute 'filter'` from inside `.contains()`.
Neither is a `CompatibilityError`/`PresetCompatibilityError`, so both
bypass the CLI's "Compatibility Error" handler in `_commands.py` and exit
1 with a raw traceback that names no field, leaving the author with no
hint which manifest key is wrong.
Type-check the field in both validators, requiring a non-empty string,
and additionally guard `check_compatibility()` in both managers since
each is public and reachable with a hand-built or mutated manifest. This
mirrors the sibling `IntegrationDescriptor`, which already requires a
non-empty string for the same key, and completes the type-checking pass
started in #3943 for the neighbouring `extension`/`preset` fields.
Adds 33 regression tests across both modules covering every escape path;
26 of them fail without this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Code (model: Claude Opus 5, supervised)
The keep-config rescue branch of install_from_directory() reads each
preserved config with bare read_bytes()/stat() calls, so a kept config
that cannot be read (permission or I/O error) crashed the reinstall
with a raw OSError. The sibling symlink guard four lines above already
rejects with a ValidationError and resolution guidance for the same
reason: bytes that cannot be safely rescued must not reach the rmtree
below.
Wrap the read and raise ValidationError with guidance, while dest_dir
is still untouched so the preserved bytes are never lost.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_script_command() split the configured command with a bare
shlex.split(), so a command string with unbalanced quotes crashed event
dispatch with a raw ValueError. The dispatcher-template twin a few
lines up already wraps the same call in try/except ValueError and
returns None so dispatch falls back cleanly.
Wrap the split the same way and return None, restoring parity between
the two paths.
Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(events): context injection for opencode and JSON-envelope agent hooks
Adds first-class context injection to agent runtime events:
1. opencode: maps session_start to experimental.chat.system.transform (injects into system prompt) and user_prompt_submit to chat.message (injects synthetic TextPart). TS plugin captures runEvent stdout (stdio pipe, encoding utf-8) and pushes into output objects. Part IDs derive from output.parts[last].id to preserve OpenCode's prt_ brand and prevent session schema crashes.
2. JSON-envelope hook wrapping: adds events_context_envelope to IntegrationBase so agents that require JSON on stdout receive their target envelope via the dispatcher's 5th argument:
- gemini, tabnine, qwen, devin: hookSpecificOutput.additionalContext on session_start/user_prompt_submit; suppress on non-injectable events (prevents systemMessage user-facing noise)
- copilot: top-level additionalContext on session_start
- cursor: top-level additional_context on session_start; suppress elsewhere
- claude, codex: plain stdout passthrough (already injected)
3. Dispatcher template and resolve_and_run_event_command parse the 5th envelope arg and wrap stdout accordingly.
Tests added for opencode TextPart schema, part ID derivation, envelope command generation, and dispatcher output wrapping. All 162 events/integration tests pass.
* fix(events): address code review on #3934
- Qwen/Gemini/Tabnine/Devin: include native hookEventName inside
hookSpecificOutput envelope (required by Qwen's hooks spec). Thread
the native event name from the integration's CANONICAL_TO_NATIVE
through _dispatcher_command as a 6th dispatcher argument, through
the dispatcher template's main()/_run_inline()/_emit(), and through
resolve_and_run_event_command()/_emit_event_stdout().
- Copilot: map user_prompt_submit to additionalContext (previously
unmapped, breaking per-prompt context injection despite Copilot CLI
supporting it via userPromptSubmitted).
- OpenCode: guard experimental.chat.system.transform so canonical
session_start handlers only run when input.sessionID is present —
OpenCode fires this hook for non-session operations (e.g. agent
generation) with no sessionID.
Assisted-by: opencode (model: glm-5.2, supervised)
* fix(events): address second Copilot review round on #3934
- Positional arg alignment: always emit default timeout (60s) as the
4th dispatcher argument even when timeout_seconds is omitted, so the
envelope (5th) and native_event (6th) land in the correct argv slots.
Previously, omitting timeout_seconds caused the envelope to be parsed
as an invalid timeout, silently falling back to plain stdout.
- OpenCode session_start caching: cache handler output per sessionID in
the generated TS plugin so non-idempotent handlers (setup, telemetry,
file-mutating scripts) run once per session instead of on every LLM
request. Cache is evicted on session.deleted.
- Updated PR description to reflect Copilot user_prompt_submit now maps
to additionalContext (was documented as plain/unprocessed).
Assisted-by: opencode (model: glm-5.2, supervised)
* fix(presets): restore core skills instead of deleting them on preset remove
Skill restoration looked for core command templates under
.specify/templates/commands, a directory specify init never populates
in real projects. Since that lookup always missed, presets overriding
a core command (e.g. speckit.plan) had their skill deleted outright on
removal instead of restored — the actual core templates live in the
bundled core_pack (wheel install) or the repo-root templates/ tree.
Restoration now falls back to that bundled location, gated behind a
restore_from_bundled_core flag so the existing "retire a stale skill
superseded by a command-mode winner" path keeps deleting rather than
resurrecting a duplicate skill.
Fixes#3928
* fix(tests): use explicit utf-8 encoding reading restored skill content
read_text() defaults to the platform locale encoding, which is cp1252
("charmap") on Windows. The bundled specify.md core template contains a
UTF-8 multi-byte emoji whose bytes aren't valid cp1252, so the Windows CI
job failed decoding the restored SKILL.md with UnicodeDecodeError.
* fix(presets): keep extension restore priority over bundled-core fallback
The bundled-core fallback added for #3928 ran before the
extension_restore_index lookup, so a skill an installed extension owns
could be silently replaced by lower-priority bundled core content on
preset removal instead of preserving the extension's winning layer.
* fix(manifests): reject non-string metadata instead of crashing on it
`ExtensionManifest` and `PresetManifest` checked only key PRESENCE for
`id`/`name`/`version`/`description`, then fed the values straight to
`re.match()` and `packaging.Version()`. Both raise a bare `TypeError` on a
non-string, which is neither `ValidationError` nor `PresetValidationError`,
so it escaped every caller that already handles a malformed manifest.
YAML makes this an easy authoring slip rather than a contrived one: an
unquoted `version: 1.0` parses as a float and `id: 2` as an int.
The user-visible symptom is the one the in-tree comment above the section
guards was written to prevent (#3898 for presets, and its extension twin):
`list_installed()` degrades a bad manifest to "⚠️ Corrupted extension" but
catches only the domain error, so a single bad manifest made
`specify extension list` / `specify preset list` exit 1 with a raw
traceback and *no output at all* — hiding every healthy extension/preset
too, not just the broken one.
Also unguarded on the same path:
- extension `provides.commands[].name` → `TypeError` from the command-name
pattern match. The sibling `file` field was already safe, since
`relative_extension_path_violation()` rejects a non-string.
- preset `provides.templates[].name`/`.file` → `TypeError` from `re.match`
and `os.path.normpath` respectively.
The third manifest twin, `IntegrationDescriptor`, is already hardened: it
type-checks the same four fields and catches `TypeError` alongside
`InvalidVersion`. This brings the other two in line with it.
Tests: 68 added across both suites, covering each field against float, int,
None, list, dict, and bool, plus an end-to-end guard per manifest type
asserting a healthy entry still lists while the bad one degrades. All 68
fail with the source change reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: narrow exception in invoke separator resolution and add regression test
Narrow 'except Exception' to 'except (ImportError, ValueError, KeyError)'
in register_commands() invoke separator resolution. Add regression test
that verifies TypeError propagates instead of being silently swallowed.
* fix: remove duplicate pass statement in agents.py
Remove redundant second pass statement in the except block
for invoke separator resolution. The narrowed exception handler
now has a single clean pass statement.
Assisted-by: GitHub Copilot (model: mimo-v2-free, supervised)
The step documents the default twice:
class docstring: "Because workflows run unattended, the step defaults to
``--ignore-agent-tools``"
field docs: "Skip checks for the coding agent CLI (defaults to ``true``)"
It implements that with `config.get("ignore_agent_tools", True)`, which
applies the default only when the key is ABSENT. A bare
`ignore_agent_tools:` in YAML parses to None, and `_resolve_bool(None)`
returns False:
key ABSENT -> True flag emitted: YES
bare ignore_agent_tools: -> False flag emitted: NO <-- bug
explicit true -> True flag emitted: YES
explicit false -> False flag emitted: NO
So the flag is dropped, `specify init` re-runs the agent-CLI presence
check, and an unattended run fails with "Agent Detection Error" for any
integration whose CLI is not installed on the runner.
Normalize an explicit null to the default, mirroring the while/do-while
`max_iterations` handling.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat: allow overriding default init integration via SPECKIT_INTEGRATION_DEFAULT
Resolve the non-interactive/init default integration from the
SPECKIT_INTEGRATION_DEFAULT environment variable, fitting the existing
SPECKIT_INTEGRATION_* namespace. Falls back to the hardcoded "copilot"
default when unset, and warns to stderr (rather than silently falling
back) when the value is not a registered integration key.
Wires the resolver into specify init (interactive prompt default and
non-interactive fallback), the init workflow step, and the bundle init
default. Adds unit and CLI tests and documents the variable.
Closes#3939
Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0
* test: cover env-var default wiring for picker, workflow step, and bundle
Address PR review: add regression tests so each SPECKIT_INTEGRATION_DEFAULT
wiring site cannot silently revert to the hardcoded constant.
- init.py: interactive picker receives the resolved key as default_key.
- workflow init step: no step/workflow default + env var drives output
integration and argv.
- bundle _resolve_init_integration: env-var default applies when unspecified,
while explicit override and manifest-declared integration still win.
Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0
* feat(extensions): scaffold config templates on extension add/enable
Deploy an extension's provides.config templates into .specify/ when the
extension is added or enabled. Existing files are never overwritten, so
user customizations are preserved.
Addresses the review on #2000:
- ExtensionManifest.config returns [] unless provides.config is a list of
dicts, so a malformed manifest cannot crash callers.
- scaffold_config returns a consistent (deployed, skipped_existing, failed)
tuple on every path, including a missing manifest.
- Template paths must resolve inside the extension dir and targets inside
.specify/; symlinks and non-regular files are rejected.
- Callers distinguish "already exists (preserved)" from "not scaffolded",
and extension_enable no longer crashes on a corrupt manifest.
- Tests cover traversal, absolute paths, symlinks, directory templates,
malformed provides.config, and the missing-manifest tuple shape.
Ported onto the extensions package introduced by #3014: the manager and
manifest changes land in extensions/__init__.py and the CLI wiring in
extensions/_commands.py.
* fix(extensions): deploy config where it is read, and contain the write
Addresses @Copilot's review.
Config now lands in .specify/extensions/<id>/ rather than the .specify/
root. ConfigManager._get_project_config() reads
.specify/extensions/<id>/<id>-config.yml, and the bundled scripts and
READMEs use the same path, so a scaffolded git-config.yml was being
written somewhere the git extension never looks.
Containment is checked component by component before .specify is used as
the root. Resolving it first and trusting the result let a symlinked
component point outside the project, after which every target satisfied
relative_to and copy2 wrote externally. This matches the project
safe-write path in shared_infra.
mkdir moved inside the OSError handler. A nested target like
foo/config.yml raised out of scaffolding when its parent could not be
created, and on extension add that happened after the extension was
already installed.
The 'Configuration may be required' warning is now conditional. It ran
unconditionally after the scaffolding block, so it contradicted the
success output directly above it and fired for extensions with no
provides.config at all.
Tests cover the corrected location, a symlinked config root, and an
uncreatable nested target.
* fix(extensions): only scaffold config targets that removal preserves
remove(keep_config=True) rmtree's every subdirectory and keeps only top-level
-config.yml / -config.local.yml files; the backup path globs the same
top-level pattern. Scaffolding a nested or differently-named target therefore
handed the user a file that 'extension add --force' silently deleted and
replaced with the template default, losing customization.
Constrain scaffold targets to that convention rather than widening four
removal paths.
---------
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
execute() reads `on_reject = config.get("on_reject", "abort")` and, in the
reject branch, handles only "abort" and "retry" before falling through to
its `# on_reject == "skip"` case. So any other value makes a REJECTED gate
report COMPLETED and the run walks straight past the review the gate
exists to enforce:
on_reject='abort' -> failed "Gate rejected by user at step 'g'"
on_reject='retry' -> paused
on_reject='skip' -> completed (by design)
on_reject='Abort' -> completed <-- rejection silently discarded
on_reject='fail' -> completed <-- same
on_reject='stop' -> completed <-- same
on_reject=None -> completed <-- same
on_reject=5 -> completed <-- same
Reachable by a capitalisation slip, a guessed verb, a non-string, or a
bare `on_reject:` — note `config.get(k, default)` does NOT substitute the
default for an explicit YAML null.
`validate` already rejects anything outside abort/skip/retry, but the
engine does not auto-validate before execute(). Fail loudly instead,
mirroring the `options` and `verdict_input` guards in the same method, and
placed before the non-TTY short-circuit so it surfaces in CI too.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: eliminate TOCTOU race in zip packaging
Open file once and derive both stat info and content from the same file
descriptor to prevent race conditions where the file is modified between
stat() and read_bytes() calls.
* test: add regression test for TOCTOU stat/read consistency in packager
The old implementation called file_path.stat() then file_path.read_bytes()
as separate syscalls. The fix opens the file once and uses os.fstat() +
fh.read() on the same handle. This test verifies the archived bytes and
mode are consistent with the opened file descriptor.
execute() did:
output_config = config.get("output") or {}
if not isinstance(output_config, dict):
output_config = {}
so every non-mapping `output` was silently discarded and the step still
returned COMPLETED — every declared aggregation key vanished, and
downstream `{{ steps.<id>.output.<key> }}` resolved to None and
interpolated as an empty string:
output=[] -> completed, error=None
output=False -> completed, error=None
output=0 -> completed, error=None
output='' -> completed, error=None
output=['a'] -> completed, error=None
output='oops' -> completed, error=None
output=5 -> completed, error=None
`validate` already rejects this and its comment names the flaw exactly:
"execute() silently coerces a non-mapping output to {}, so the author's
declared aggregation keys would vanish with no error." The engine does not
auto-validate before execute(), so on an unvalidated run that is what
happened — and `x or {}` masked the falsy shapes before the isinstance
check even ran.
Fail loudly with validate()'s own message, mirroring the `wait_for` guard
in the same method. An explicit `output:` (YAML null) stays valid.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add --extension flag to specify init for installing extensions at init time
Adds a repeatable --extension flag to `specify init` so users can opt into
extensions (bundled name, local path, or HTTPS URL) during initialization,
without a separate `specify extension add` step.
- New `_install_extension_during_init` helper in commands/init.py that
auto-detects source type (URL / local path / bundled name / catalog) and
installs via ExtensionManager. Failures are non-fatal and recorded in the
tracker without aborting init.
- Extension tracker steps are pre-registered before the Live context and run
after preset install, before finalize.
- Five new tests in TestExtensionFlag covering bundled name, multiple
extensions, local absolute path, unknown extension (graceful error), and
combination with --preset.
Rebased onto upstream/main and adapted to the refactored init command
(moved to src/specify_cli/commands/init.py) from stale PR #2396.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Address review: reuse hardened downloader, refresh events, escape labels, fix bundler call
Responds to review feedback on #3914 and fixes CI (pytest bundler failure).
- Extract shared `install_extension_from_url` helper in extensions/_commands.py
that reuses the authenticated, redirect-guarded, bounded (50 MiB) download
and TOCTOU-safe transient archive used by `extension add --from`. Both
`extension add --from` and `specify init --extension <url>` now go through
this single downloader instead of a second raw urlopen path.
- Refresh native event configuration once after successful extension installs
during init (mirrors `_refresh_events_and_warn` in the add path) so an
extension declaring `events:` has its hooks activated.
- Escape user-controlled extension specs and error text before interpolating
them into StepTracker labels (Rich markup injection).
- Pass `extensions=None` from bundler's `_run_init` so the init callback no
longer receives the typer OptionInfo sentinel ('OptionInfo' object is not
iterable), which broke `test_install_initializes_uninitialized_project`.
- Add init URL coverage in TestExtensionFlag: non-HTTPS rejection and a
successful HTTPS ZIP install with download-cache cleanup assertion.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add default-deny trust confirmation for URL extension installs at init
URL-based --extension installs now require explicit trust, matching the
`extension add --from` posture. Interactive sessions show an "Untrusted
Source" panel and prompt (default no); non-interactive sessions deny by
default unless --trust-extension-urls is passed. Trust is resolved before
the Live display since the prompt can't be answered under the spinner.
- Add --trust-extension-urls option and _ext_spec_is_url /
_confirm_extension_url_trust helpers
- Skip (not abort) unconfirmed URL extensions, consistent with other
non-fatal extension failures
- Pass trust_extension_urls=False from the bundler init callback
- Add tests for deny-by-default, interactive confirm, and trusted install
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bc6802d-81b8-48f4-8f60-cba3aebc3bb3
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bc6802d-81b8-48f4-8f60-cba3aebc3bb3
* fix: bound response reads in extension catalog and download
Replace unbounded
esponse.read() calls with
ead_response_limited()
from _download_security in extensions/__init__.py to prevent denial-
of-service via oversized catalog or extension archive responses.
Three call sites fixed:
- _fetch_single_catalog JSON read (catalog metadata)
- _fetch_catalog JSON read (legacy path)
- download_extension ZIP read (binary download)
All existing mock tests updated to use side_effect with BytesIO.read
instead of
eturn_value, ensuring compatibility with the chunked read
loop in
ead_response_limited.
Two regression tests added:
- test_oversized_catalog_response_rejected
- test_oversized_extension_download_rejected
* fix: remove .decode utf-8 to preserve bytes for json.loads
json.loads accepts bytes directly. Removing .decode maintains
compatibility with BOM-bearing or UTF-16/32 catalogs.
A gate with `on_reject: retry` consumes a bound reject verdict before
pausing by resetting the named input to `""` (documented behaviour, so a
later resume prompts again). Every `resume()` re-resolves the persisted
inputs through `_coerce_input`.
Those two rules collide when the bound input declares an `enum` that does
not list `""`. The reset writes a value the input's own enum forbids, and
the run wedges:
inputs:
spec_verdict:
type: string
enum: [approve, reject]
steps:
- id: review
type: gate
options: [approve, reject]
on_reject: retry
verdict_input: spec_verdict
$ specify workflow run wf --input spec_verdict=reject
Status: paused
$ specify workflow resume <run_id> --input note=b
Error: Input 'spec_verdict' value '' not in allowed values:
['approve', 'reject'].
The workflow validates clean and the first run looks fine, so the failure
only appears at the second resume. It is also unrecoverable in practice:
`_resolve_inputs` re-coerces the whole persisted map, so *any* resume that
supplies an input dies on the stored `""`. Only a resume with no inputs at
all still works -- and that is precisely the call that cannot deliver a new
verdict, which is the one thing the retry cycle exists to allow.
Extend the existing `verdict_input` cross-check (which already confirms the
name is declared) to also require that a retry-bound input's `enum` admits
the reset sentinel, and report it with a fix hint. To do that, thread the
input *definitions* through `_validate_steps` instead of just their names.
Rejected the alternative of popping the key instead of writing `""`: that
lets the input's `default` flow back in on the next resume, so a gate the
user just rejected would silently auto-approve.
Docs: note the `enum` requirement next to the reset behaviour it follows
from.
Adds 4 validation tests for the new guard plus a characterization test that
drives the engine directly to pin the wedge it prevents.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Opus 5 (1M context)
`workflow resolve` printed two lines through `console.print`, which has
Rich markup enabled, without escaping:
1. The layer tier was wrapped in literal brackets:
`f" • [{layer.tier}] {layer.source} ..."`. Rich parsed `[base]`
and `[project-overlay]` as style tags, so the tier label was swallowed
on *every* invocation -- no untrusted input required. The column has
never rendered.
2. Step attribution interpolated `composed.step_id` raw. Step IDs come
from base-workflow / overlay YAML and are only validated against `:`
(see `_parse_edit`), so brackets pass validation. A balanced
`[stuff]` is silently swallowed; an unbalanced `[/red]` raises
`rich.errors.MarkupError`, producing an uncaught traceback and exit 1
-- the workflow cannot be inspected at all.
Route the interpolated fields through `rich.markup.escape` and escape
the literal tier bracket as `\[`, matching the existing pattern in
`workflow info`'s step graph and `workflow_list`'s `\[disabled]`.
Only display is affected; the returned payload was already unescaped and
is unchanged.
Adds 3 regression tests, all of which fail without the fix: the tier
label renders, and a step ID survives both the swallowing and the
crashing markup cases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Opus 5 (1M context)
* fix(scripts): tolerate an unusable integration.json in the Python helper
`get_invoke_separator()` in scripts/python/common.py indexed the parsed JSON
directly, so two shapes escaped its `except (OSError, json.JSONDecodeError)`
while BOTH of its twins fall back to "." for them:
* A non-mapping top level is valid JSON, so JSONDecodeError never fires and
`state.get(...)` raised AttributeError.
* A non-UTF-8 file raises UnicodeDecodeError -- a ValueError, not an OSError.
Realistic on Windows, where PowerShell 5.1's Out-File/`>` default to UTF-16.
Measured on main -- 6 of 7 inputs crashed the Python helper while bash and
PowerShell 5.1 returned "." for every one:
input python bash pwsh 5.1
{"default_integration":"forge"} '.' . .
[] AttributeError . .
"forge" AttributeError . .
42 AttributeError . .
null AttributeError . .
UTF-16 file UnicodeDecodeError . .
Split the parse out of the lookup, complete the exception tuple, and guard the
top-level shape -- matching `read_feature_json_feature_directory` in this same
module, which already does exactly this. The hyphen-separator feature is
unchanged (regression test included).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(scripts): point the parity comment at the sibling above, not below
read_feature_json_feature_directory is defined at line 81, above
get_invoke_separator, so "below" sent maintainers the wrong way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Noor ul ain