Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Dispose rejected or redirected OpenAPI response bodies, including header
validation and redirect-limit failures. Stop streaming unused bodies after
reporting an error. Keep normal complete responses and load limits unchanged.
- Keep lock preflight errors separate from rename contention so a denied
observation cannot silently admit a transaction. Preserve the existing
bounded retry for Windows lock observations and propagate other failures.

- Retry a denied lock-directory scan during Windows publication handoff within
the existing wait bound. Also retry denied owner-record reads and lock
observations while a prior owner removes them. Retain the native error when
Expand Down
2 changes: 2 additions & 0 deletions docs/0.5.0-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ The executed matrix covers two simultaneous stale-owner observations for file an

On Windows, inspecting a lock directory or opening its owner record while its previous owner removes it can return `EPERM`. The acquisition loop retries read-only lock observations (`lstat`, `scandir`, `open`, `read`) within its existing ten-second bound and retains the native cause if denial persists; write/deletion failures and other filesystem errors still propagate. A repeated four-writer generator probe reproduced native scan and owner-open failures on separate source heads. Deterministic file/directory cases verify transient recovery, permanent-denial timeout, exact bytes and owner preservation; separate deletion-denial cases verify those errors propagate without entering a transaction.

Lock preflight and rename now have separate error classification. A one-shot `EACCES` during preflight propagates without entering a transaction, preserving its exact owner record and file bytes. OpenAPI loading destroys unused response bodies before rejecting headers/status or following a redirect. The executed matrix and tested process boundaries are recorded in [0.5.0 hardening](0.5.0-hardening.md).

## Release boundaries

The normal packed consumer checks both retained declaration names, nine rejected old imports and private paths under strict TypeScript 6 and 7, then runs the installed CLI help/version commands. No production dependency, package version or peer range changes in this cut. Full candidate graph, generated website API and maintainer review still gate release. CLI hardening issue #159 stays open until its remaining qualification is complete.
21 changes: 21 additions & 0 deletions docs/0.5.0-hardening.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# 0.5.0 CLI hardening

Issue [#159](https://github.com/askrjs/askr-cli/issues/159) covers CLI failure boundaries. These are executed assertions and reproduced failures. Package versions remain unchanged during preparation; the final candidate graph and maintainer review are separate release gates.

| Invariant | Executed evidence | Outcome and boundary |
| ------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Failed file updates preserve original data and report recoverable copies. | [File transaction failures](../tests/file-transaction-failures.test.ts), [file changes](../tests/file-changes.test.ts), [update CLI](../tests/update-cli.test.ts); 17 added cases in [#176](https://github.com/askrjs/askr-cli/pull/176), 13 failing pre-fix assertions. | Partial writes and failed replacement/rollback/cleanup are covered. POSIX file modes are preserved; five mode assertions are skipped on Windows. Actual process termination retains original copies for manual restoration. Automatic file restoration after abrupt termination is not claimed. |
| Directory publication recovers before the next operation. | [Directory swaps](../tests/directory-swap.test.ts) and [termination worker](../tests/fixtures/directory-publication-crash-worker.ts); 24 added cases in [#177](https://github.com/askrjs/askr-cli/pull/177), six failing pre-fix assertions. | Actual termination at prepared, backed-up and published checkpoints; failed rollback and partial cleanup; corrupt records, conflicting trees, missing trees and junctions. Recovery preserves originals or complete new output. Ambiguous state fails with retained recovery paths. |
| Generation and create preserve destination ownership. | [Generation publication](../tests/generation-publication.test.ts) and its [fault worker](../tests/fixtures/generation-publication-worker.ts); 17 added cases in [#178](https://github.com/askrjs/askr-cli/pull/178), twelve failing pre-fix assertions. | Generator shares directory recovery. Partial stages are cleaned, a cleanup failure names its retained stage, and partial backup deletion does not roll back complete output. Concurrent creates have one winner and preserve its unrelated files. Success follows publication. |
| Contending commands do not delete another lock owner. | [Lock ownership](../tests/filesystem-locks.test.ts), [lock termination worker](../tests/fixtures/filesystem-lock-crash-worker.ts), repeated [generation](../tests/generate.test.ts); [#179](https://github.com/askrjs/askr-cli/pull/179), [#180](https://github.com/askrjs/askr-cli/pull/180), [#181](https://github.com/askrjs/askr-cli/pull/181). | Unique tokens, delayed reapers, release handoff, stale writes, legacy recovery, ambiguous contents, junctions, preparation collision and cleanup failures are covered. Two processes are actually killed before lock publication. A repeated four-writer case checks 200 publications and complete final output. Earlier Windows scan/open failures remain in CI history; corrected PR and merge checks passed on all three platforms. |
| Read failures are classified at the operation that failed. | [Lock preflight and observation assertions](../tests/filesystem-locks.test.ts). | Native `EPERM` observations retry within the existing ten-second bound, preserving the cause if permanent. A one-shot preflight `EACCES` propagates without entering either a file or directory transaction. Rename contention and deletion denials retain their separate handling. |
| Rejected remote downloads stop consuming their bodies. | Extended [remote transport tests](../tests/generate-remote.test.ts); separate real local HTTPS probe under Node 24. | Encoding, declared size, redirect limit, missing Location, HTTP error and cross-origin redirect failures destroy unused bodies. A successful redirect preserves its complete final response. Chunk limits, body deadline and abortion also destroy the response. Before the fix, five real HTTPS cases retained one or two sockets and continued streaming after rejection; after the fix, all five closed with no subsequent body writes. This real HTTPS probe ran locally; hosted assertions use controlled transport. |
| Malformed input fails before mutation or registry access. | Nineteen parameterized [update CLI cases](../tests/update-cli.test.ts) and separate real command probes. | Truncated/null/array/scalar manifests; malformed workspaces and update policy; invalid pnpm YAML and package declarations. Every file name and byte remains unchanged, and registry calls stay at zero. Real malformed SSG module and missing-registry probes preserve existing published output. |
| Actual installer process outcomes preserve publication boundaries. | Five cases in [generation publication](../tests/generation-publication.test.ts), using the [create worker](../tests/fixtures/create-process-worker.ts) and local installer shim. | Missing command, nonzero exit, externally terminated installer, successful installer and abruptly terminated CLI are exercised through real `spawnSync`. Failure exits preserve unrelated data, report no success, publish no destination and remove their private project stage. Success publishes the complete prepared project. Killing the CLI retains an unpublished private stage; the test explicitly stops its owned installer and removes its own fixture. |
| Path pivots and repeated commands preserve unrelated files. | [Generate](../tests/generate.test.ts), [remote transport](../tests/generate-remote.test.ts), [discovery](../tests/update-discovery.test.ts), [CLI smoke](../tests/cli-smoke.test.ts) and the failure suites above. | Unsafe project names, file/junction destinations, output containing input, local reference traversal, remote-to-file pivots, allowed origins, source limits, workspace boundaries, dry-run immutability, stale plans and concurrent skill copies are exercised. Existing assertions remain in the full suite. |

The final read-failure pass adds eleven cases and strengthens an existing remote assertion. Ten assertions fail against immutable pre-fix source: two preflight denials and eight response-disposal cases. Nineteen malformed-input cases and five actual installer cases extend the existing suites without adding command features.

Validation uses Node LTS: `npm run check`, `npm run bench`, `npm run test:templates`, `npm run test:peer-floor`, `npm run fmt -- --check` and production dependency audit. The normal packed consumer checks two retained declaration names, nine removed imports and fifteen private paths under actual TypeScript 6.0.3 and 7.0.2, then invokes installed help/version. Packed template integration performs normal installs for all five starters. The unchanged performance suite has 22 gates.

Parent-process coverage does not measure the lines executed by termination workers. The fault assertions and actual process checkpoints provide separate evidence. Tests of current cooperating lock owners do not qualify concurrent use of older recursive-lock protocols. Process termination does not establish power-loss durability. The installer has no automatic timeout contract: externally stopping a real installer and injecting an `ETIMEDOUT` result are covered, but an automatic installer timeout is not claimed.
46 changes: 23 additions & 23 deletions src/filesystem-lock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,32 +118,32 @@ export async function acquireFilesystemLock(
if (lockStat && (!lockStat.isDirectory() || lockStat.isSymbolicLink())) {
throw ambiguousLock(lock);
}
await fs.rename(stage, lock);
return async () => {
await fs.unlink(path.join(lock, ownerName));
// Another contender can already have replaced the empty old lock with
// its nonempty lock. Never recursively remove a successor's contents.
await removeEmptyLock(lock);
};
} catch (error) {
const code = error instanceof Error && "code" in error ? String(error.code) : "";
if (!RENAME_CONTENTION_CODES.has(code)) throw error;
lastError = error;
try {
if (await removeOrphanedLock(lock)) continue;
} catch (inspectionError) {
// Windows can deny reads while another owner removes its lock record.
// Retry within the same deadline; a permanent denial retains its cause.
if (
!isNodeError(inspectionError, "EPERM") ||
!LOCK_READ_SYSCALLS.has(inspectionError.syscall ?? "")
) {
throw inspectionError;
}
lastError = inspectionError;
await fs.rename(stage, lock);
return async () => {
await fs.unlink(path.join(lock, ownerName));
// Another contender can already have replaced the empty old lock with
// its nonempty lock. Never recursively remove a successor's contents.
await removeEmptyLock(lock);
};
} catch (error) {
const code = error instanceof Error && "code" in error ? String(error.code) : "";
if (!RENAME_CONTENTION_CODES.has(code)) throw error;
lastError = error;
}
await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_MS));
if (await removeOrphanedLock(lock)) continue;
} catch (inspectionError) {
// Windows can deny reads while another owner removes its lock record.
// Keep read errors distinct from rename contention and deletion failures.
if (
!isNodeError(inspectionError, "EPERM") ||
!LOCK_READ_SYSCALLS.has(inspectionError.syscall ?? "")
) {
throw inspectionError;
}
lastError = inspectionError;
}
await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_MS));
}
} catch (error) {
try {
Expand Down
20 changes: 10 additions & 10 deletions src/generate/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,17 +357,17 @@ async function responseText(
maxBytes: number,
deadline: number,
): Promise<string> {
const declared = Number(response.headers["content-length"]);
if (Number.isFinite(declared) && declared > maxBytes)
throw new GenerationError(`OpenAPI reference exceeds ${maxBytes} bytes: ${uri}`);
const encoding = response.headers["content-encoding"];
if (encoding && encoding !== "identity")
throw new GenerationError(
`OpenAPI reference returned unsupported content encoding: ${encoding}`,
);
const chunks: Uint8Array[] = [];
let size = 0;
try {
const declared = Number(response.headers["content-length"]);
if (Number.isFinite(declared) && declared > maxBytes)
throw new GenerationError(`OpenAPI reference exceeds ${maxBytes} bytes: ${uri}`);
const encoding = response.headers["content-encoding"];
if (encoding && encoding !== "identity")
throw new GenerationError(
`OpenAPI reference returned unsupported content encoding: ${encoding}`,
);
await withDeadline(
new Promise<void>((resolve, reject) => {
response.message.on("data", (value: Buffer) => {
Expand Down Expand Up @@ -416,17 +416,17 @@ async function fetchSource(uri: string, options: ResolvedLoadOptions): Promise<S
const addresses = await vettedAddresses(current, options, deadline);
const response = await requestVetted(current, addresses, deadline);
if (response.status >= 300 && response.status < 400) {
response.message.destroy();
if (redirects >= options.maxRedirects)
throw new GenerationError(`Too many redirects while fetching OpenAPI reference ${uri}`);
response.message.resume();
const location = response.headers.location;
if (!location)
throw new GenerationError(`OpenAPI redirect is missing Location: ${current.href}`);
current = new URL(location, current);
continue;
}
if (response.status < 200 || response.status >= 300) {
response.message.resume();
response.message.destroy();
throw new GenerationError(
`Unable to fetch OpenAPI reference ${current.href}: ${response.status} ${response.statusText}`,
);
Expand Down
Loading
Loading