diff --git a/.gitignore b/.gitignore
index ffafb61..890f7d5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,4 @@
node_modules/
dist/
.worktrees
-docs/
tasks/
diff --git a/CLAUDE.md b/CLAUDE.md
index abda29a..850259f 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -35,26 +35,27 @@ bun run format # Prettier
- Every `@clack/prompts` call must check `p.isCancel()` and exit gracefully.
- `shell.ts` reads stdout and stderr concurrently with `Promise.all` to avoid pipe deadlocks.
-## Comment discipline
-
-**Default: write zero comments.** Well-named identifiers + control flow explain WHAT. This project follows the global "no comments unless genuinely complex" rule **strictly** — stricter than the global default.
-
-**Fix rationale belongs in commit messages, not code.** "We added X to prevent Y outage" is commit-message content. It does NOT go in the code — comments drift from the implementation as the code evolves, commit messages and PR descriptions don't.
-
-**The only code comments that earn their keep are footgun warnings** — ones that save a future dev from a specific non-obvious trap AND aren't findable from git blame. Examples that pass the bar:
-
-- `// Bun.spawnSync returns null exitCode on timeout kill.` (runtime quirk)
-- `// Constant-time compare prevents timing side-channel on hash compare.` (security invariant the call site alone doesn't communicate)
-- `// POSIX setsid(): survives terminal close so a slow download isn't SIGHUPed.` (cross-platform behavior note)
-
-**Anti-patterns — NEVER write any of these** (concrete examples from real commits that violated this rule):
-
-1. **Paraphrasing the next line** — `// Bump throttle on transient network failures so we don't burn the GitHub API quota` above `recordCheckCompleted();`. The function name already says this.
-2. **JSDoc-style docblocks for internal helpers** — `// true=exists, false=ENOENT, null=non-ENOENT error logged; caller must bail` above `function checkExists(...): boolean | null`. The return type plus null-checks at call sites already communicate the contract.
-3. **Multi-line fix rationale** — any 2+ line comment explaining WHY a PR-level decision was made. That belongs in the commit message. If it's not findable from `git blame`, improve the commit message instead of polluting the code.
-4. **Stacked WHY paragraphs** — back-to-back `// line 1 / // line 2 / // line 3` blocks. Treat 2 lines as a warning sign; 3+ lines is always wrong.
-
-**Self-test before writing any comment**: remove it and re-read the function. Would a future reader (including future-me) be meaningfully more confused without it? If the answer is "no" or "barely" — delete the comment.
+## Code comments
+
+> **No comments in code. Rename or restructure instead.** Two exceptions, and each must fit on one line:
+>
+> 1. A `/** */` docstring on an exported symbol, stating a contract the signature cannot show.
+> 2. A citation for a constraint the code cannot express: a URL, a spec section, an ADR or `docs/` path, or an issue number. The line carries the pointer, never the explanation.
+>
+> Everything else is banned. That includes any comment that explains what the code does, why it has its shape, what would break, or what you learned while writing it. Put that reasoning in the PR body, a test name, or an ADR.
+>
+> Existing comments in a file are not a style to match and not a license to add more. Leave them alone when you touch the file for another reason.
+>
+> Before reporting a code change done, print every added comment line that lacks a citation token and delete each comment it shows. The pattern is a net, not a parser: a printed line that is not a comment is a false positive to leave alone.
+>
+> ```bash
+> CITED='https?://|docs/|ADR|#[0-9]+|§'
+> git diff -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.go' '*.rs' '*.java' '*.kt' '*.swift' '*.c' '*.h' '*.cpp' '*.cs' \
+> | grep -E '^\+([[:space:]]*(//|/\*|\* )|.*[[:space:]](//|/\*))' \
+> | grep -vE "$CITED|^\+[[:space:]]*/\*\*.*\*/[[:space:]]*\$"
+> git diff -U0 -- '*.py' '*.sh' '*.zsh' '*.rb' '*.toml' '*.yml' '*.yaml' \
+> | grep -E '^\+([[:space:]]*#|.*[[:space:]]#)' | grep -vE "$CITED|^\+#!"
+> ```
## Dependencies
diff --git a/docs/adr_auto_update_security.md b/docs/adr_auto_update_security.md
new file mode 100644
index 0000000..abe9a6c
--- /dev/null
+++ b/docs/adr_auto_update_security.md
@@ -0,0 +1,77 @@
+# ADR: auto-update security model
+
+Status: accepted.
+
+Worktree-cli self-updates by downloading compiled binaries from GitHub
+releases in a background child process and swapping them in on the next
+launch. The updater fetches over the network, writes executables, and runs
+them, so every input is untrusted until verified.
+
+## 1. Threat model
+
+Attackers considered: a network adversary off GitHub origins, a compromised
+CDN or release asset, a tampered SHA256SUMS file, a local user planting
+symlinks in shared install dirs, and stale or rolled-back stages.
+Non-goals: defending a machine whose running binary is already compromised,
+and hiding version numbers.
+
+## 2. Host pinning and redirects
+
+Fetches go only to allowlisted GitHub hosts: api.github.com, github.com,
+codeload.github.com, objects.githubusercontent.com,
+release-assets.githubusercontent.com, and
+github-releases.githubusercontent.com. The check compares URL.host and does
+not require HTTPS. Redirects are followed manually so each hop's host is
+validated before connecting. Authorization is stripped once the host differs
+from the first URL's host and never re-added, so a chain that bounces back
+to the starting host cannot re-attach the token; a same-host scheme change
+does not strip it. Redirect refusals log the host only, because signed CDN
+URLs can carry tokens in the query string, while the initial
+disallowed-host refusal logs a truncated URL.
+
+## 3. Size caps
+
+Asset downloads are capped at 200 MB, which is headroom over the current
+~50 MB binary, and oversized responses are rejected before verification.
+Chunks stream to disk with the cap enforced as bytes arrive instead of
+buffering the whole body in memory.
+
+## 4. Release metadata and checksums
+
+Release tags must match a strict version pattern before they are used in
+paths or logs. The SHA256SUMS parser lowercases hex, skips blanks and
+`#` comments, rejects BSD-tagged `SHA256 (file) = hex` lines, and treats
+duplicate entries as tampering. Tamper (parsed but malformed sums) is a
+distinct outcome from fetch errors: tamper escalates loudly and burns the
+throttle, while fetch errors retry when transient. Retryable statuses are
+5xx plus 403 and 429, which are the GitHub rate-limit signals; other 4xx are
+permanent. In the background path, releases without SHA256SUMS fall back to
+a self-hash recorded in the sidecar, which detects local stage-to-apply
+corruption only, not upstream tampering; the foreground updater instead
+proceeds without hash verification and says so.
+Requests identify as worktree-cli and use GITHUB_TOKEN when present, since
+authenticated calls get a far higher rate limit than anonymous ones.
+
+## 5. Staging and apply integrity
+
+Temp and sidecar files are pre-unlinked so writes cannot follow planted
+symlinks. Verification runs before chmod and before the probe, because
+executing an unverified binary is code execution. The probe requires the
+staged binary to run `--version` successfully with version-shaped output,
+because a hash match does not prove runnability, and it runs with
+auto-update disabled so the probe cannot spawn grandchildren or consume a
+stale stage. The sidecar writer is locked to the reader's version and hash
+pattern so a future parser relaxation cannot turn a crafted tag into a hash
+spoof. A stage older than the running version is discarded as stale, since a
+foreground update may have raced a background check, and applying it would
+silently downgrade. Hash comparison is constant-time to close the timing
+side channel, and the sums object has a null prototype to block `__proto__`
+pollution from a tampered file.
+
+## 6. Throttle policy
+
+A completed check burns the 24h throttle window on structural or permanent
+outcomes, so a broken release does not cost a download or API call on every
+launch. Transient outcomes keep retrying: a missing arch asset (the
+maintainer may upload it later), transient sums errors, local hash I/O
+errors, and sidecar or stage writes that fail for non-permission reasons.
diff --git a/docs/comment_salvage.md b/docs/comment_salvage.md
new file mode 100644
index 0000000..6f7d9d6
--- /dev/null
+++ b/docs/comment_salvage.md
@@ -0,0 +1,47 @@
+# Comment salvage
+
+Facts removed from code comments because the no-comments rule allows only
+one-line citations. Each entry names the file and symbol it came from. Prune
+this file by verifying each fact and either citing a source or deleting it.
+
+## src/lib/editor.ts / resolveEditor
+
+- clack's select returns string | symbol, but isCancel narrows the symbol
+ case above.
+
+## src/lib/git.ts / selectWorktree
+
+- p.select returns string | symbol, but isCancel above exits on symbol, and
+ library types do not narrow.
+
+## src/lib/auto-update.ts / scheduleBackgroundUpdateCheck
+
+- POSIX setsid(): survives terminal close so a slow download isn't SIGHUPed.
+- Close parent's fd copy even if Bun.spawn throws synchronously (else fd
+ leak per launch).
+
+## src/lib/auto-update.ts / probeBinaryRuns
+
+- Bun.spawnSync returns null exitCode on timeout kill.
+
+## src/lib/auto-update.ts / decodeProbeStream
+
+- Emit a debuggable marker (not "") so a Bun API shape change is visible in
+ last-error.
+
+## src/lib/config.ts / readConfigFile
+
+- file.exists() can throw on stat errors.
+
+## src/lib/config.ts / shouldAutoUpdate
+
+- file.exists() can throw EACCES.
+
+## src/lib/fs-utils.ts / classifyWriteError
+
+- Walks cause chain for errno; EBUSY/ETXTBSY treated as permanent (file
+ locked/busy).
+
+## src/lib/release.ts / withTimeout
+
+- Drain the redirect body so keep-alive sockets don't pin across hops.
diff --git a/src/commands/internal-update-check.ts b/src/commands/internal-update-check.ts
index a8e01e6..f2139d4 100644
--- a/src/commands/internal-update-check.ts
+++ b/src/commands/internal-update-check.ts
@@ -10,7 +10,6 @@ export const internalUpdateCheckCommand = command({
desc: "",
hidden: true,
handler: async () => {
- // Detached child's stderr is redirected; catch so panics still hit last-error.
try {
await runBackgroundUpdateCheck();
} catch (error) {
diff --git a/src/commands/update.ts b/src/commands/update.ts
index a36901b..d0382bd 100644
--- a/src/commands/update.ts
+++ b/src/commands/update.ts
@@ -85,7 +85,7 @@ export const updateCommand = command({
printInfo(`Downloading ${assetName}...`);
const tmpPath = `${binaryPath}.update-tmp`;
- // Pre-unlink to prevent symlink-follow in shared install dirs.
+ // docs/adr_auto_update_security.md §5
await safeUnlink(tmpPath);
const { error: dlError } = await tryCatch(
downloadAsset(asset, tmpPath)
@@ -150,7 +150,7 @@ export const updateCommand = command({
process.exit(EXIT_CODES.ERROR);
}
- // Probe before rename — SHA match ≠ runnable; segfaults on libc/codesign mismatch.
+ // docs/adr_auto_update_security.md §5
const probe = probeBinaryRuns(tmpPath);
if (!probe.ok) {
await safeUnlink(tmpPath);
@@ -177,7 +177,7 @@ export const updateCommand = command({
process.exit(EXIT_CODES.ERROR);
}
- // Invalidate pending stage + bump throttle to prevent silent downgrade on next launch.
+ // docs/adr_auto_update_security.md §5, §6
cleanupStagedArtifacts();
recordCheckCompleted();
diff --git a/src/index.ts b/src/index.ts
index 1da84c5..9e59a35 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -20,7 +20,6 @@ const META_FLAGS = new Set(["--version", "-v", "--help", "-h"]);
const FOREGROUND_UPDATE_SUBCOMMAND = "update";
function isMetaInvocation(): boolean {
- // Match only the first positional arg so flag-as-value (e.g. `create my-feature -h`) still auto-updates.
const first = process.argv[2];
return first !== undefined && META_FLAGS.has(first);
}
@@ -28,7 +27,6 @@ function isMetaInvocation(): boolean {
function shouldSkipAutoUpdate(): boolean {
const first = process.argv[2];
if (first === INTERNAL_CHECK_SUBCOMMAND) return true;
- // Skip for the foreground updater to avoid racing its own binary install.
if (first === FOREGROUND_UPDATE_SUBCOMMAND) return true;
return isMetaInvocation();
}
@@ -37,14 +35,12 @@ if (!shouldSkipAutoUpdate()) {
try {
applyPendingUpdate();
} catch (error) {
- // Never crash the entry point — the user's command (including `worktree update`) must still run.
appendBackgroundCheckPanic(error);
const { DIM, RESET } = COLORS;
console.error(
`${DIM}worktree: auto-update apply failed unexpectedly — set WORKTREE_NO_UPDATE=1 to disable; see ~/.cache/worktree-cli/last-error${RESET}`
);
}
- // Funnel async throws into the panic logger, not an unhandled rejection.
void scheduleBackgroundUpdateCheck().catch(appendBackgroundCheckPanic);
}
diff --git a/src/lib/auto-update.ts b/src/lib/auto-update.ts
index b319270..fae6bde 100644
--- a/src/lib/auto-update.ts
+++ b/src/lib/auto-update.ts
@@ -28,7 +28,6 @@ const PROBE_STDERR_TRUNCATE_BYTES = 500;
const INTERNAL_CHECK_SUBCOMMAND = "__internal_update_check";
const SIDECAR_VERSION_PATTERN = /^\d+\.\d+\.\d+(?:-[\w.-]+)?$/;
const SIDECAR_HASH_PATTERN = /^[0-9a-f]{64}$/;
-// Defer reaping partial stages: a concurrent producer mid-commit looks identical to an orphan.
const STAGING_ORPHAN_GRACE_MS = 60 * 1000;
function getBinaryDir(): string {
@@ -172,7 +171,6 @@ function isWithinGracePeriod(filePath: string): boolean {
});
if (error) {
if (isEnoent(error)) return false;
- // Non-ENOENT: be conservative (return true) — never destroy a peer's stage on incomplete stat info.
appendLastError("apply", `grace-stat: ${error.message}`);
return true;
}
@@ -182,7 +180,6 @@ function isWithinGracePeriod(filePath: string): boolean {
function applyPendingUpdate(): void {
if (process.env.WORKTREE_NO_UPDATE === "1") return;
- // Gate on config too: a staged binary must not apply if the user set AUTO_UPDATE=false after it was staged.
const configAllows = shouldAutoUpdateSync(function (msg) {
appendLastError("apply", msg);
});
@@ -194,7 +191,6 @@ function applyPendingUpdate(): void {
const stagedExists = checkExists(stagedPath, "apply");
if (stagedExists === null) return;
if (!stagedExists) {
- // Within grace window, assume concurrent producer; past it, reap orphan.
if (isWithinGracePeriod(metaPath)) return;
safeUnlinkSync(metaPath);
return;
@@ -234,7 +230,7 @@ function applyPendingUpdate(): void {
return;
}
- // Gate against silent downgrade from a stale stage (e.g. foreground update raced a background check).
+ // docs/adr_auto_update_security.md §5
const stageCmp = compareVersions(pkg.version, meta.version);
if (stageCmp > 0) {
cleanupStagedArtifacts();
@@ -274,7 +270,6 @@ function applyPendingUpdate(): void {
fs.renameSync(stagedPath, process.execPath);
});
if (renameError) {
- // Persistent rename failures won't self-heal; cleanup to avoid looping on every launch.
cleanupStagedArtifacts();
const writeCode = classifyWriteError(renameError);
const rawCode = (renameError as NodeJS.ErrnoException).code;
@@ -294,7 +289,6 @@ function applyPendingUpdate(): void {
return;
}
safeUnlinkSync(metaPath);
- // Bump throttle so the sibling scheduleBackgroundUpdateCheck doesn't redundantly re-check.
recordCheckCompleted();
const { GREEN, BOLD, RESET } = COLORS;
@@ -302,7 +296,6 @@ function applyPendingUpdate(): void {
`worktree ${GREEN}${BOLD}auto-updated${RESET} to ${BOLD}v${meta.version}${RESET}`
);
} catch (error) {
- // Swallow errno-style I/O only; let programmer bugs propagate with a stack trace.
if (!(error instanceof Error) || !("code" in error)) {
throw error;
}
@@ -345,7 +338,6 @@ async function readLastCheckMs(): Promise {
async function isAutoUpdateDisabled(): Promise {
if (process.env.WORKTREE_NO_UPDATE === "1") return true;
- // Fail CLOSED on broken config so a typo can't silently disable auto-update.
return !(await shouldAutoUpdate(function (msg) {
appendLastError("check", msg);
}));
@@ -354,7 +346,6 @@ async function isAutoUpdateDisabled(): Promise {
async function scheduleBackgroundUpdateCheck(): Promise {
try {
if (!isStandalone()) return;
- // Skip spawn if cache is unwritable; the child would also fail and burn API quota.
if (hasCacheWriteFailed) return;
if (await isAutoUpdateDisabled()) return;
@@ -366,8 +357,6 @@ async function scheduleBackgroundUpdateCheck(): Promise {
now - lastCheck < TWENTY_FOUR_HOURS_MS;
if (shouldSkip) return;
- // Only the child writes last-check on success, so a failed check never burns the 24h window.
- // Child stderr is funneled to last-error so background panics are visible on the next launch.
const { data: stderrFd, error: stderrOpenError } = tryCatchSync(
function () {
ensureCacheDir();
@@ -375,8 +364,6 @@ async function scheduleBackgroundUpdateCheck(): Promise {
}
);
if (stderrOpenError) {
- // If we can't capture the child's stderr, don't spawn blind — the
- // throttle cache lives in the same dir, so it's likely unwritable too.
hasCacheWriteFailed = true;
warnCacheWriteFailureOnce(stderrOpenError.message);
return;
@@ -387,18 +374,15 @@ async function scheduleBackgroundUpdateCheck(): Promise {
stdin: "ignore",
stdout: "ignore",
stderr: stderrFd,
- // POSIX setsid(): survives terminal close so a slow download isn't SIGHUPed.
detached: true,
}).unref();
} finally {
- // Close parent's fd copy even if Bun.spawn throws synchronously (else fd leak per launch).
const inheritedFd = stderrFd;
tryCatchSync(function () {
fs.closeSync(inheritedFd);
});
}
} catch (error) {
- // Swallow errno-style only; let programmer bugs propagate.
if (!(error instanceof Error) || !("code" in error)) {
throw error;
}
@@ -413,7 +397,6 @@ function recordCheckCompleted(): void {
fs.writeFileSync(getLastCheckPath(), String(Date.now()));
});
if (error) {
- // Latch: future calls and scheduleBackgroundUpdateCheck short-circuit.
hasCacheWriteFailed = true;
appendLastError("check", `last-check write: ${error.message}`);
warnCacheWriteFailureOnce(error.message);
@@ -423,7 +406,6 @@ function recordCheckCompleted(): void {
async function runBackgroundUpdateCheck(): Promise {
const assetName = getAssetName();
if (!assetName) {
- // Structural — burn throttle so we don't thrash the API.
appendLastError("check", `unsupported platform/arch`);
recordCheckCompleted();
return;
@@ -449,7 +431,6 @@ async function runBackgroundUpdateCheck(): Promise {
return entry.name === assetName;
});
if (!asset) {
- // Transient: maintainer may upload the missing arch later; don't burn throttle.
appendLastError(
"check",
`release ${release.tag} missing asset ${assetName}`
@@ -463,7 +444,7 @@ async function runBackgroundUpdateCheck(): Promise {
`${STAGING_FILENAME}.${randomBytes(8).toString("hex")}.tmp`
);
- // Pre-unlink to prevent the write from following a planted symlink.
+ // docs/adr_auto_update_security.md §5
safeUnlinkSync(tmpPath);
const { error: dlError } = await tryCatch(
downloadAsset(asset, tmpPath, undefined, function (op, downloadErr) {
@@ -477,7 +458,7 @@ async function runBackgroundUpdateCheck(): Promise {
return;
}
- // Verify BEFORE chmod/probe: running an unverified binary is code execution.
+ // docs/adr_auto_update_security.md §5
const verify = await verifyAssetAgainstSums(
tmpPath,
assetName,
@@ -496,7 +477,6 @@ async function runBackgroundUpdateCheck(): Promise {
"check",
`SHA256SUMS fetch failed — refusing to stage: ${verify.reason}`
);
- // Burn throttle for permanent failures; transient ones keep retrying.
if (!verify.retryable) {
recordCheckCompleted();
}
@@ -507,7 +487,6 @@ async function runBackgroundUpdateCheck(): Promise {
);
recordCheckCompleted();
} else if (verify.kind === "hash-io-error") {
- // Local IO may be transient (disk full mid-write); don't burn throttle.
appendLastError(
"check",
`hash io-error for ${assetName}: ${verify.cause.message}`
@@ -536,12 +515,11 @@ async function runBackgroundUpdateCheck(): Promise {
if (!probe.ok) {
safeUnlinkSync(tmpPath);
appendLastError("check", `probe: ${probe.reason}`);
- // Probe fail is structural for this release — burn throttle or we redownload 50 MB every launch.
recordCheckCompleted();
return;
}
- // Legacy release lacks SHA256SUMS; self-hash only detects local stage→apply corruption, not upstream tampering.
+ // docs/adr_auto_update_security.md §4
if (verifiedHash === null) {
const { data: computed, error: hashError } = tryCatchSync(function () {
return computeSha256Sync(tmpPath);
@@ -557,7 +535,7 @@ async function runBackgroundUpdateCheck(): Promise {
verifiedHash = computed;
}
- // Lock writer to reader's pattern so a future parser relaxation can't turn a crafted tag into a hash-spoof.
+ // docs/adr_auto_update_security.md §5
if (!SIDECAR_VERSION_PATTERN.test(release.version)) {
safeUnlinkSync(tmpPath);
appendLastError(
@@ -583,7 +561,6 @@ async function runBackgroundUpdateCheck(): Promise {
safeUnlinkSync(tmpPath);
safeUnlinkSync(metaTmpPath);
appendLastError("check", `sidecar write: ${metaWriteError.message}`);
- // Structural permission/readonly errors won't self-heal; burn throttle.
if (classifyWriteError(metaWriteError) !== null) {
recordCheckCompleted();
}
@@ -626,11 +603,11 @@ function probeBinaryRuns(filePath: string): ProbeResult {
const { data: result, error } = tryCatchSync(function () {
return Bun.spawnSync({
cmd: [filePath, "--version"],
- // Capture stdout to reject exit-0-with-garbage as a valid probe.
+ // docs/adr_auto_update_security.md §5
stdout: "pipe",
stderr: "pipe",
timeout: PROBE_TIMEOUT_MS,
- // Disable auto-update in the probe to prevent grandchild spawn / stale-stage consumption.
+ // docs/adr_auto_update_security.md §5
env: { ...process.env, WORKTREE_NO_UPDATE: "1" },
});
});
@@ -641,7 +618,6 @@ function probeBinaryRuns(filePath: string): ProbeResult {
};
}
if (result.exitCode === null) {
- // Bun.spawnSync returns null exitCode on timeout kill.
return {
ok: false,
reason: `timed out after ${PROBE_TIMEOUT_MS}ms`,
@@ -665,7 +641,6 @@ function probeBinaryRuns(filePath: string): ProbeResult {
function decodeProbeStream(stream: unknown): string {
if (!(stream instanceof Uint8Array) && !(stream instanceof Buffer)) {
- // Emit a debuggable marker (not "") so a Bun API shape change is visible in last-error.
return ``;
}
const bytes = stream instanceof Buffer ? new Uint8Array(stream) : stream;
diff --git a/src/lib/config.ts b/src/lib/config.ts
index a3657a4..7a10d9e 100644
--- a/src/lib/config.ts
+++ b/src/lib/config.ts
@@ -93,7 +93,6 @@ async function readConfigFile(
): Promise {
const file = Bun.file(filePath);
const display = displayPath(filePath);
- // file.exists() can throw on stat errors — guard like shouldAutoUpdate below.
const { data: isExists, error: existsError } = await tryCatch(
file.exists()
);
@@ -115,7 +114,6 @@ async function readConfigFile(
}
const raw = parseConfigContent(content);
if (scope === "project" && "AUTO_UPDATE" in raw) {
- // Also validate — user moving this line to ~/.worktreerc later needs to know if it's syntactically valid.
const { data: probe, error: probeError } = tryCatchSync(function () {
return booleanLike.safeParse(raw.AUTO_UPDATE);
});
@@ -129,7 +127,6 @@ async function readConfigFile(
`${filePath}:AUTO_UPDATE`,
`warning: AUTO_UPDATE in project ${display} is ignored — set it in ~/.worktreerc instead${validityNote}.`
);
- // Strip pre-validate so `AUTO_UPDATE=junk` doesn't discard valid sibling keys.
delete raw.AUTO_UPDATE;
}
const { data: parsed, error: parseError } = tryCatchSync(function () {
@@ -167,12 +164,9 @@ function decideAutoUpdateFromContent(
return parsed.AUTO_UPDATE;
}
-// Fail CLOSED on parse/read errors so a typo can't silently override opt-out.
-// `onError` threads diagnostics so users discover *why* auto-update is disabled.
async function shouldAutoUpdate(onError?: AutoUpdateOnError): Promise {
const filePath = path.join(os.homedir(), ".worktreerc");
const file = Bun.file(filePath);
- // `file.exists()` can throw EACCES; guard to avoid crashing the scheduler.
const { data: isExists, error: existsError } = await tryCatch(
file.exists()
);
@@ -189,7 +183,6 @@ async function shouldAutoUpdate(onError?: AutoUpdateOnError): Promise {
return decideAutoUpdateFromContent(content, onError);
}
-// Sync twin used at startup by applyPendingUpdate (before brocli.run / top-level await).
function shouldAutoUpdateSync(onError?: AutoUpdateOnError): boolean {
const filePath = path.join(os.homedir(), ".worktreerc");
const { data: isExists, error: existsError } = tryCatchSync(function () {
diff --git a/src/lib/editor.ts b/src/lib/editor.ts
index 8ef397f..7737127 100644
--- a/src/lib/editor.ts
+++ b/src/lib/editor.ts
@@ -44,7 +44,6 @@ async function resolveEditor(preferred?: string): Promise {
process.exit(EXIT_CODES.ERROR);
}
- // clack's select returns string | symbol, but isCancel narrows the symbol case above
return choice as EditorChoice;
}
diff --git a/src/lib/fs-utils.ts b/src/lib/fs-utils.ts
index 755b781..7ac4080 100644
--- a/src/lib/fs-utils.ts
+++ b/src/lib/fs-utils.ts
@@ -36,7 +36,6 @@ function safeUnlinkSync(filePath: string): void {
type WriteErrorCode = "EACCES" | "EPERM" | "EROFS" | "EBUSY" | "ETXTBSY";
-// Walks cause chain for errno; EBUSY/ETXTBSY treated as permanent (file locked/busy).
const WRITE_ERROR_CODES = new Set([
"EACCES",
"EPERM",
@@ -59,7 +58,6 @@ function classifyWriteError(error: unknown): WriteErrorCode | null {
return null;
}
-// Unwrap `cause` to surface the original errno message instead of a generic wrapper.
function deepestMessage(error: unknown): string {
let cur: unknown = error;
while (cur instanceof Error && cur.cause !== undefined) {
diff --git a/src/lib/git.ts b/src/lib/git.ts
index 772e305..a8a2e23 100644
--- a/src/lib/git.ts
+++ b/src/lib/git.ts
@@ -517,7 +517,6 @@ async function selectWorktree(
process.exit(EXIT_CODES.SUCCESS);
}
- // p.select returns string | symbol, but isCancel above exits on symbol — library types don't narrow
return selected as string;
}
diff --git a/src/lib/release.test.ts b/src/lib/release.test.ts
index 5d02a2a..20f9ef6 100644
--- a/src/lib/release.test.ts
+++ b/src/lib/release.test.ts
@@ -39,18 +39,15 @@ describe("compareVersions", () => {
});
it("orders prerelease tags per SemVer 2.0 §11", () => {
- // Numeric within-identifier comparison.
expect(compareVersions("1.2.3-beta.1", "1.2.3-beta.2")).toBeLessThan(0);
- // Lex on string identifiers.
expect(compareVersions("1.2.3-rc.1", "1.2.3-beta.1")).toBeGreaterThan(
0
);
- // Equal prereleases.
expect(compareVersions("1.2.3-alpha", "1.2.3-alpha")).toBe(0);
});
it("compares numeric prerelease identifiers numerically (SemVer 2.0)", () => {
- // SemVer 2.0 §11.4.1: numeric identifiers compare numerically — rc.10 > rc.2.
+ // SemVer 2.0 §11.4.1
expect(compareVersions("1.2.3-rc.10", "1.2.3-rc.2")).toBeGreaterThan(0);
expect(compareVersions("1.2.3-rc.2", "1.2.3-rc.10")).toBeLessThan(0);
expect(compareVersions("1.2.3-alpha.9", "1.2.3-alpha.11")).toBeLessThan(
@@ -59,16 +56,14 @@ describe("compareVersions", () => {
});
it("treats numeric identifiers as lower precedence than string identifiers", () => {
- // SemVer 2.0 §11.4.3: numeric identifiers always have lower precedence than
- // alphanumeric identifiers within the same prerelease position.
+ // SemVer 2.0 §11.4.3
expect(
compareVersions("1.0.0-alpha.1", "1.0.0-alpha.beta")
).toBeLessThan(0);
});
it("longer prerelease wins when all preceding identifiers equal", () => {
- // SemVer 2.0 §11.4.4: a larger set of fields has higher precedence than
- // a smaller set, when all preceding identifiers are equal.
+ // SemVer 2.0 §11.4.4
expect(compareVersions("1.0.0-alpha", "1.0.0-alpha.1")).toBeLessThan(0);
expect(
compareVersions("1.0.0-alpha.beta", "1.0.0-alpha.beta.1")
@@ -154,7 +149,6 @@ describe("parseSha256Sums", () => {
it("rejects BSD-tagged-format `SHA256 (file) = hex` (not the format we publish)", () => {
const text = `SHA256 (worktree-darwin-arm64) = ${"a".repeat(64)}`;
const result = parseSha256Sums(text);
- // Pins the parser to reject unknown formats — guards against accepting unverified hashes.
expect(Object.keys(result)).toEqual([]);
});
});
@@ -258,7 +252,6 @@ describe("verifyAssetAgainstSums", () => {
const ASSET_BYTES = new Uint8Array([
0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64,
]);
- // Precomputed SHA256 of ASSET_BYTES.
const ASSET_SHA =
"b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9";
const ASSET_NAME = "worktree-darwin-arm64";
@@ -280,12 +273,11 @@ describe("verifyAssetAgainstSums", () => {
try {
fs.unlinkSync(tmpFile);
} catch {
- // ignore
+ return;
}
});
function makeAsset(name: string): ReleaseAsset {
- // Allowlisted host so the withTimeout host-pin doesn't reject pre-stub.
return {
name,
browser_download_url: `https://objects.githubusercontent.com/${name}`,
@@ -377,7 +369,7 @@ describe("verifyAssetAgainstSums", () => {
if (result.ok) return;
expect(result.kind).toBe("sums-error");
if (result.kind !== "sums-error") return;
- // 403/429 are GitHub rate-limit signals — transient, NOT permanent.
+ // docs/adr_auto_update_security.md §4
expect(result.retryable).toBe(true);
});
@@ -413,9 +405,6 @@ describe("verifyAssetAgainstSums", () => {
]);
expect(result.ok).toBe(false);
if (result.ok) return;
- // Distinct kind from "sums-error" so foreground/background paths can
- // escalate (loud red error / TAMPER: log prefix) instead of treating
- // tampering the same as a transient outage.
expect(result.kind).toBe("sums-tamper");
if (result.kind !== "sums-tamper") return;
expect(result.reason).toMatch(/Duplicate/);
diff --git a/src/lib/release.ts b/src/lib/release.ts
index 2ce6b60..b6d7eb8 100644
--- a/src/lib/release.ts
+++ b/src/lib/release.ts
@@ -8,7 +8,7 @@ import pkg from "../../package.json";
const REPO = "bhagyamudgal/worktree-cli";
const API_RELEASES_LATEST = `https://api.github.com/repos/${REPO}/releases/latest`;
-// Host-pin fetches to GitHub origins; defense-in-depth against CDN/release-asset compromise.
+// docs/adr_auto_update_security.md §2
const ALLOWED_RELEASE_HOSTS = new Set([
"api.github.com",
"github.com",
@@ -28,7 +28,7 @@ function isAllowedReleaseHost(urlString: string): boolean {
const RELEASE_TAG_PATTERN = /^v?\d+\.\d+\.\d+(?:-[\w.-]+)?$/;
-// Identify as worktree-cli; GITHUB_TOKEN bumps rate limit from 60/hr to 5000/hr.
+// docs/adr_auto_update_security.md §4
function buildGitHubHeaders(): Record {
const headers: Record = {
"User-Agent": `worktree-cli/${pkg.version}`,
@@ -44,7 +44,7 @@ function buildGitHubHeaders(): Record {
const DEFAULT_META_TIMEOUT_MS = 30_000;
const DEFAULT_ASSET_TIMEOUT_MS = 600_000;
-// 4× headroom over current ~50 MB binary; rejects oversized CDN responses pre-verification.
+// docs/adr_auto_update_security.md §3
const MAX_ASSET_BYTES = 200 * 1024 * 1024;
const MAX_REDIRECT_HOPS = 5;
@@ -103,7 +103,7 @@ function parseVersion(v: string): ParsedVersion {
};
}
-// SemVer 2.0 §11: pairwise compare; numeric(
controller.abort();
}, timeoutMs);
try {
- // Follow redirects manually so each hop's host is validated BEFORE we connect to it —
- // default `redirect: "follow"` connects to intermediate hosts and only exposes the final URL.
+ // docs/adr_auto_update_security.md §2
const originHost = new URL(url).host;
let currentUrl = url;
- // Once Authorization has been stripped on any cross-origin hop, never re-add —
- // prevents a redirect chain that bounces back to the origin host from re-attaching the token.
+ // docs/adr_auto_update_security.md §2
let authStripped = false;
for (let hop = 0; hop < MAX_REDIRECT_HOPS; hop++) {
const headers = buildGitHubHeaders();
@@ -200,7 +198,6 @@ async function withTimeout(
});
if (response.status >= 300 && response.status < 400) {
const location = response.headers.get("location");
- // Drain the redirect body so keep-alive sockets don't pin across hops.
await tryCatch(response.body?.cancel() ?? Promise.resolve());
if (!location) {
throw new Error(
@@ -209,7 +206,7 @@ async function withTimeout(
}
const next = new URL(location, currentUrl).toString();
if (!isAllowedReleaseHost(next)) {
- // Log host only, not the full URL — signed CDN URLs can carry tokens in the query string.
+ // docs/adr_auto_update_security.md §2
throw new Error(
`Refused redirect to disallowed host: ${new URL(next).host}`
);
@@ -241,7 +238,7 @@ async function fetchLatestRelease(
if (!isReleaseInfo(json)) {
throw new Error("Release payload missing tag_name or assets");
}
- // Reject malformed tags at the boundary so they can't propagate into paths/logs.
+ // docs/adr_auto_update_security.md §4
if (!RELEASE_TAG_PATTERN.test(json.tag_name)) {
throw new Error(
`Release tag malformed: ${JSON.stringify(json.tag_name.slice(0, 40))}`
@@ -301,8 +298,7 @@ async function downloadAsset(
`Download ${asset.name} refused: empty response body`
);
}
- // Stream chunks directly to disk, enforcing the cap as bytes arrive.
- // Avoids the ~2× memory peak of buffering all chunks then copying into one final Uint8Array.
+ // docs/adr_auto_update_security.md §3
const reader = response.body.getReader();
const writer = fs.createWriteStream(destPath, { flags: "w" });
let bytesReceived = 0;
@@ -323,7 +319,6 @@ async function downloadAsset(
}
}
if (bytesReceived === 0) {
- // Explicit empty-body error; else SHA verify later reports a misleading mismatch.
throw new Error(
`Download ${asset.name} refused: empty response body`
);
@@ -352,7 +347,6 @@ async function downloadAsset(
)
);
if (error) {
- // Clean up our own partial write so callers don't have to do it defensively.
const { error: cleanupError } = tryCatchSync(function () {
fs.unlinkSync(destPath);
});
@@ -433,7 +427,7 @@ function verifyBinaryHashSync(
function constantTimeEquals(a: string, b: string): boolean {
if (a.length !== b.length) return false;
- // Constant-time compare prevents timing side-channel on hash compare.
+ // docs/adr_auto_update_security.md §5
return timingSafeEqual(Buffer.from(a), Buffer.from(b));
}
@@ -441,10 +435,10 @@ type Sha256SumsResult =
| { kind: "not-published" }
| { kind: "ok"; sums: Record }
| { kind: "error"; reason: string; retryable: boolean }
- // "tamper" = parsed-but-malformed sums (today: duplicates) — distinct from transient "error".
+ // docs/adr_auto_update_security.md §4
| { kind: "tamper"; reason: string };
-// 5xx and 403/429 (rate-limit) retryable; other 4xx treated as permanent.
+// docs/adr_auto_update_security.md §4
function isRetryableHttpStatus(status: number): boolean {
if (status === 403 || status === 429) return true;
return status >= 500 && status < 600;
@@ -478,7 +472,7 @@ async function fetchSha256Sums(
retryable: true,
};
}
- // Duplicate entries are tampering, not transient — permanent failure.
+ // docs/adr_auto_update_security.md §4
const { data: parsed, error: parseError } = tryCatchSync(
function () {
return parseSha256Sums(text);
@@ -505,7 +499,7 @@ async function fetchSha256Sums(
}
type VerifyAssetResult =
- | { ok: true; hash: string | null } // hash === null when SHA256SUMS isn't published
+ | { ok: true; hash: string | null }
| { ok: false; kind: "sums-error"; reason: string; retryable: boolean }
| { ok: false; kind: "sums-tamper"; reason: string }
| { ok: false; kind: "missing-entry" }
@@ -551,7 +545,7 @@ async function verifyAssetAgainstSums(
}
function parseSha256Sums(text: string): Record {
- // Null-prototype object blocks __proto__/constructor pollution from a tampered file.
+ // docs/adr_auto_update_security.md §5
const result: Record = Object.create(null);
for (const line of text.split("\n")) {
const trimmed = line.trim();