mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-02 05:24:34 +08:00
Clay GoodandClaude Opus 5
6295515d4d
* fix(update): flag a stale global CLI during openspec update Instruction files are generated by the installed CLI, so running `openspec update` against an outdated global install printed "All 1 tool(s) up to date (v1.6.0)" while the workflows newer releases ship were never written. Users read that as success and reported the missing workflows as bugs. `openspec update` now checks the npm registry alongside the update and, when the installed CLI is behind, prints the upgrade command instead of leaving the up-to-date line to speak for itself. The check never gets in the way: it runs concurrently with the update, times out after 1.5s, caches the answer for 24h, returns null on any failure, and is skipped in CI, under tests, and whenever OPENSPEC_NO_UPDATE_CHECK is set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): name the right install in the stale-CLI hint The hint assumed a global install. A project-local dependency is now pointed at that dependency instead of `npm install -g`, and every hint prints the directory the running CLI was loaded from, so anyone who upgraded but still runs an old pnpm/volta/npx shim can see which copy answered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): drop the temp-file cache and fix prerelease ordering CodeQL flagged the version-check cache twice: a predictable path in the shared OS temp dir (js/insecure-temporary-file, high) and registry data written to that file (js/http-to-file-access, medium). `openspec update` is a rare, human-run command, so the cache bought little — removing it resolves both alerts outright and deletes the code that needed them. Also from review: CI=1 now opts out alongside CI=true, and prerelease tags compare per SemVer (dot-separated identifiers, numeric compared numerically) so 1.7.0-beta.10 outranks 1.7.0-beta.2. Build metadata is ignored per spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): make the version check actually reach the registry Adversarial review found the check could never fire: the request sent `accept: application/vnd.npm.install-v1+json`, which npm serves only on the full packument — on `/<pkg>/latest` it answers 406, so every real run returned null. Every test mocked fetch, so nothing caught it. The header is gone, and a new suite exercises the real fetch path against a local HTTP server, including an assertion that we never send that Accept type. Also from review: - Validate the published version against a strict SemVer pattern before printing it. It lands in the terminal beside an install command, so an unvalidated string could smuggle ANSI cursor controls and repaint the surrounding lines. - Honor DO_NOT_TRACK=1 and OPENSPEC_TELEMETRY=0, the opt-outs telemetry already respects, and update SECURITY.md, which promised telemetry was the only network egress. - Anchor project-local detection on the path being updated and its ancestors instead of process.cwd(), so `openspec update <path>` and workspace sub-packages with a hoisted root node_modules are no longer told to install globally. It can no longer throw when the working directory has been deleted. - Send npx/dlx users `npx @fission-ai/openspec@latest update` rather than advice that would create the global install they avoided. - Query npm_config_registry when set, so private mirrors get an answer their own install command can deliver. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(update): make version-check fixtures portable on Windows The new fixtures mixed unresolved POSIX literals with path.join output. On Windows path.resolve adds a drive letter and path.join does not, so the prefix match could never succeed and two assertions failed there. Fixtures now derive from resolved roots. Real installs were unaffected — both sides come from resolved absolute paths — but case and drive-letter casing can still differ between require.resolve and path.resolve on Windows, so the comparison is now case-insensitive on win32. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): stop a blackholed registry from holding the CLI open Verification found the 1.5s timeout did not bound the command. Aborting a fetch still completing its TCP handshake — a firewall dropping packets, a captive portal — leaves the connect handle ref'd, so `openspec update` sat for ~10s after printing everything. Measured against an unroutable address: resolved at 1523ms, process exited at 10558ms. The request now uses node:http(s), whose socket the timeout can actually destroy: same probe resolves at 1547ms and exits at 1550ms. Because the client is no longer fetch, the mocked tests would have gone inert and silently reached the real registry. The whole suite now drives the real code path against a local server, which is also the only way to prove an opt-out sent nothing. Added a child-process guard for the teardown itself (no in-process assertion can see it), a case for a non-JSON body — the captive-portal login page — and order-independence fixes: the mock leak between describes made the 406 regression guard the first casualty under --sequence.shuffle. Also: bound the version pattern and the response body so neither can be absurdly long, and narrow the ephemeral-runner match so a user directory named "dlx" is no longer mistaken for a pnpm cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(update): offer to run the upgrade instead of only printing it Being told to run a command, then run the update again, is two steps the CLI can take for you. `openspec update` now asks: A newer OpenSpec CLI is available (v1.6.0 -> v1.7.0). Running from: /usr/local/lib/node_modules/@fission-ai/openspec ? Upgrade to v1.7.0 now? (Y/n) Yes runs `npm install -g` with stdio inherited — so any auth or sudo prompt reaches the user directly — then re-runs the update with the new CLI, because this process still holds the old templates and cannot write the new workflows itself. No prints the command and updates with the CLI you have. It asks rather than acting: a CLI that mutates a global install without consent is the wrong default. Guards: - Interactive terminals only, via the repo's isInteractive() (no TTY, or CI set, means the note prints exactly as before). - Global npm installs only. A project dependency belongs to that project's package manager, and an npx/dlx cache has nothing to upgrade; both get the command instead. - The re-run carries OPENSPEC_NO_UPDATE_CHECK=1, so a PATH that still resolves to the old binary cannot loop. - A failed upgrade, a missing openspec on PATH, and Ctrl-C at the prompt each fall back to the printed command rather than an error. The check now runs before the update rather than alongside it, so an accepted upgrade regenerates files with the new templates in one pass. Verified end to end against a stubbed npm and openspec on PATH, both answers, plus the unchanged non-interactive path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): offer the upgrade only where npm install -g would help Review found `canSelfUpgrade` treated "not a project dependency and not an npx cache" as proof of a global npm install. It is not: a pnpm, bun, yarn or volta global, and a plain git clone, all qualified. Reproduced by running the CLI from this repo — it offered to npm install -g over the checkout, which would have shadowed it with a second copy. The offer now requires npm to own the install, derived from the running node's global root (and APPDATA/npm_config_prefix) rather than by shelling out to `npm prefix -g`. Everything else gets the command that matches how it was installed — `pnpm add -g`, `bun add -g`, `yarn global add`, `volta install` — a project dependency is pointed at its own package manager with no npm command at all, and a source checkout gets no note, since its version is whatever the branch says. Docs corrected where they had drifted from the code: - The check runs before the update, not alongside it; it can delay the update by up to 1.5s. docs/cli.md and the changeset said otherwise. - npm_config_registry is only honored when npm exports it; an .npmrc setting alone is invisible to us. Docs and JSDoc claimed more. - SECURITY.md gains an "Installing software" row: running a package manager on the user's behalf is the most security-relevant behavior here and the table did not mention it. The "Running other programs" row now covers the re-run's path argument and cross-spawn's Windows shim escaping, and the network row lists every opt-out precisely. - troubleshooting.md's "Commands don't show up" — the exact symptom this PR exists to fix — now explains that instruction files come from the installed CLI, and installation.md's Updating section links onward. - The env-var table notes the CI and NODE_ENV skips, and that npm_config_registry must be an http(s) URL. - "the new workflows land in the same command" no longer overpromises: when the upgraded openspec is not on PATH, the CLI now says the files were not regenerated instead of printing a dim aside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): make the upgrade offer tell the truth about what happened Adversarial review found the flow could claim success it had not earned, and could strand a non-interactive caller. All verified by running the CLI, all fixed: - `npm install -g` exits 0 even when it installs nothing, so "✓ Upgraded to vX" was an assertion, not a fact. The version is now read back from the installed binary; when another install earlier on PATH still answers with the old one — the exact silent staleness this feature exists to fix — it says so instead of claiming the upgrade landed. - The prompt hung forever under `openspec update > log.txt`: the question went to the file while the user watched a blank terminal. The offer now requires stdout to be a terminal too. - Ctrl-C at the prompt read as "no thanks" and carried on into the next prompt. It now stops the command with 130. - `--force` never reached the re-run, so `openspec update --force` could regenerate nothing and exit 0. Flags are forwarded, with `--` before the path so a flag-shaped path stays a path. - A signal-killed re-run, and a re-run with no CLI to hand off to, both reported 0. Both now report failure. - `process.exit()` skipped commander's postAction hook, killing the telemetry flush mid-request. The action sets process.exitCode and returns instead. - The check read only npm_config_registry, which npm exports only under `npm run` — so an enterprise user with a mirror in .npmrc got an unannounced call to public npm. It now reads .npmrc too. - Two different CI predicates: `CI=yes` suppressed the prompt but not the request. One predicate now, and it treats any value except an explicit off-value as CI. - A project-local install was offered a global one when updating a different directory; both anchors are checked now. Tests: the re-run had no coverage at all and now has four cases. Mutation testing over nine mutations (406 header, DO_NOT_TRACK, version validation, prerelease ordering, canSelfUpgrade, the anti-loop env guard, the cwd-vs-target anchor, the timeout) — one survived, the anti-loop guard, so it has a test now and the mutation dies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(update): compare re-run arguments as tokens, not as a raw line cmd.exe echoes `%*` with every argument quoted, so the Windows job saw `"update" "--force" "--" "--weird-path"` and the substring assertion for `-- --weird-path` failed. The forwarding itself was correct on both platforms; the assertion now splits and unquotes before checking that the separator immediately precedes the path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): read only the user's .npmrc for the registry CodeQL flagged file data reaching an outbound request, and it has a point: the project `.npmrc` travels with the repository, so honoring it let a cloned repo choose where the version check sends its request. Only `~/.npmrc` is read now — which is where a mirror is configured anyway, since `npm config set registry` writes there — and a test pins that a project `.npmrc` cannot redirect the request. Docs and changeset say so explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): detect the install from its own layout, not from node's path Adversarial review found the offer never appeared on Homebrew — a mainstream macOS install — and I reproduced it on this machine: npm root -g: /opt/homebrew/lib/node_modules derived roots: /opt/homebrew/Cellar/node/25.8.1_1/lib/node_modules process.execPath is realpath'd through Homebrew's symlink into the Cellar, so a root derived from the node binary never matches the prefix npm installs into. The same mismatch hits Debian-style layouts. The install's own shape is now the primary signal: <prefix>/lib/node_modules/<pkg> (POSIX) or <prefix>/node_modules/<pkg> (Windows), confirmed by the bin directory npm would have written the shim into. The node-derived roots stay as a fast path. Also from the same review, each reproduced first: - volta nests a whole node install, so its packages sit in exactly npm's layout: we called it npm-owned, ran `npm install -g`, and on failure told the user to run volta. Ownership is now decided before location. - upgradedBinPath returned the first prefix that merely had an openspec in it, preferring a stale one over the prefix npm just wrote to. It now derives from the running install first. - readCliVersion took the first version-shaped token anywhere in stdout, so a wrapper banner ("Node.js v25.8.1 | OpenSpec") was read as the answer — turning a real upgrade into a false "still reports vX", or worse, claiming success for a version nobody installed. It now takes the line that is only a version. - The probe child could outlive its 5s timeout indefinitely: SIGTERM with no escalation and no unref, so a signal-trapping wrapper held the CLI open for as long as it ran. - "Another install earlier on your PATH is answering first" was a misdiagnosis whenever we had asked a known binary directly. - A `registry=${VAR}` or `@scope:registry=` line in .npmrc — both npm's documented syntax, the latter being how a scoped package is normally routed to a mirror — silently fell back to the public registry. - A 3xx from the registry disabled the check permanently and silently. Redirects are followed, bounded, under one timeout budget. - An incidental directory named "pnpm" or "yarn" was read as a global install of one, printing the wrong upgrade command. Plus the earlier docs-audit round: the npx branch no longer tells users to run an update they were just handed, the check no longer fires for a source checkout whose answer is discarded, the offer gate moved into a tested pure function, and the declined command now prints below the update output instead of scrolling away above it. Docs: install-flavor table, CI off-values, empty-value opt-out, the "no cache" fact in SECURITY.md, and a changeset trimmed to a summary that points at the CLI reference. The changeset is now `minor` — this adds a prompt, an env var, an outbound request, and the ability to install software; that is not a patch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): stop reading .npmrc for the registry CodeQL flagged file data reaching an outbound request (js/file-access-to-http), and it is right that a file choosing where a request goes is a flow worth avoiding. The convenience did not earn it: reading ~/.npmrc needed three follow-up fixes in one review round (project-vs-user precedence, ${VAR} expansion, scoped registry keys), and none of it is necessary — anyone on a private mirror can export npm_config_registry, which is still honored, or turn the check off. Removes the .npmrc read and its two helpers; a test pins that a registry= line in a .npmrc cannot steer the request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
5.6 KiB
5.6 KiB
Security Policy
Reporting a vulnerability
Report privately through GitHub Security Advisories. Please don't open a public issue for a suspected vulnerability.
Include what you can: affected version, reproduction steps, and the impact you believe it has. We aim to acknowledge within 3 business days and to ship a fix or a decision within 30 days. Valid reports are credited in the advisory unless you'd rather stay anonymous.
Supported versions
Fixes ship in the latest published version on npm. Older versions are not patched — upgrade to pick up a fix.
Threat model
OpenSpec is a local command-line tool. It has no server, no network listener, and no privileged daemon. It reads and writes markdown under the directory you run it in, using paths you supply, with your own user permissions. It can offer to upgrade itself during openspec update, and only with your say-so. It sends anonymous usage telemetry, which you can disable with OPENSPEC_TELEMETRY=0.
That shapes what is and isn't a vulnerability here:
| In scope | Out of scope |
|---|---|
| Code execution triggered by parsing a spec, config, or template file | Reading or writing a file path you passed to the CLI yourself |
| Escaping the directory OpenSpec was pointed at, via untrusted input | Static-analysis findings on file-path joins with no untrusted input |
| Leaking credentials or file contents through telemetry or logs | Vulnerabilities in devDependencies that don't ship in the published package |
| Prototype pollution or injection reachable from a config or spec file | Denial of service against your own machine using your own input |
If you think something sits on the boundary, report it and we'll work it out together.
Published package contents
The openspec npm package publishes dist/, bin/, schemas/, and scripts/postinstall.js. Build and test tooling (vite, rollup, vitest, eslint, and their transitive dependencies) is not published. Scanners that read pnpm-lock.yaml without separating dependency scope will report advisories for packages that never reach an installed copy of OpenSpec.
You do not have to take that on trust — install the package and look:
npm install @fission-ai/openspec
ls node_modules | grep -E '^(vite|rollup|vitest|eslint|js-yaml|minimatch)$' # no matches
pnpm audit --prod in this repository reports the same scope, and CI runs it on every pull request.
What the CLI does on your machine
| Surface | Behavior |
|---|---|
| Install script | scripts/postinstall.js prints one line suggesting shell completions. It makes no network request, writes no files, and runs no shell. Completions are opt-in via openspec completion install. |
| Running other programs | Every call that goes through a shell uses a fixed literal (which gh, gh auth status). Anything carrying your input — issue text, editor paths, workset commands, the path passed to openspec update — uses an argument array, never string interpolation into a shell. On Windows, .cmd shims are launched through cross-spawn, which escapes arguments rather than concatenating them. |
| Installing software | openspec update can run npm install -g @fission-ai/openspec@latest and then re-run openspec update with the upgraded CLI. It does this only after you answer yes to a prompt, only for the OpenSpec package itself, only when npm owns the install, and never in CI or a non-interactive shell. A global install lives outside your project, so it runs with your permissions there and executes whatever lifecycle scripts the published package ships. It then reads the installed binary's version back rather than assuming the upgrade took. Decline and it prints the command for you to run yourself. |
| Telemetry | Command name, OpenSpec version, and a locally generated random UUID. No file paths, no file contents, no environment, no hostname, and IP capture is explicitly disabled. Opt out with OPENSPEC_TELEMETRY=0 or DO_NOT_TRACK=1; it is off in CI automatically. |
| Network | Telemetry when enabled, and one npm registry request during openspec update to check whether a newer CLI has been published. That request sends no data about you beyond what any HTTP request reveals, runs once per openspec update with nothing cached, and is skipped when CI is set to anything but an explicit off-value, under NODE_ENV=test, or when OPENSPEC_NO_UPDATE_CHECK, DO_NOT_TRACK=1, or OPENSPEC_TELEMETRY=0 is set. Reading, writing, and validating specs is entirely local. |
Automated checks
| Tool | Covers |
|---|---|
| CodeQL | Static analysis on every push and pull request to main |
| Dependabot | Dependency advisories plus weekly update pull requests for the CLI, the docs site, and CI actions |
| Dependency review | Blocks a pull request that introduces a high-severity dependency |
| Secret scanning | Enabled on the repository, including push protection |
pnpm audit |
Published dependencies are audited on every pull request, on pushes to main, and weekly. Advisory on pull requests so an unrelated change is not blocked; failing elsewhere, so a new advisory surfaces even when no dependency changed. Build tooling is always advisory. |
| Pinned actions | Every GitHub Action runs from a commit SHA, so a moved tag cannot change what CI executes |
Alerts are triaged against the threat model above, so a finding in build-only tooling is fixed on the normal update cadence rather than treated as an incident.