diff --git a/README.md b/README.md index e234facc..bf2d7a31 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ What `sync` does with a `{{key}}` in a server, at schema 4: The message names the ingredient, the field and the kind — `mcp/idp: the value rendered into server.headers.Authorization looks like a credential (github-token) — …` — and never the value. The way out is to keep the secret out of a param: have the server read it from the environment, and declare the variable in `authEnv` so `doctor` and the example settings file name it. A URL whose userinfo is made only of `${NAME}` references, such as `https://${IDP_USER}:${IDP_TOKEN}@idp.example.com/`, is let through. `doctor`'s `mcp-env` checks the names it finds in the rendered server's `env` and in `authEnv`; it reads no other field. -The check narrows the road; it does not close it. **Still passes, though it is a secret:** a token of an unknown format (outside `env` and `args` nothing looks for it; inside them it can only warn, and only when the whole value is letters, digits and `+ / = _ -`) — so `Bearer `, `KEY: ` and a `…;Password=…` connection string pass when the token's format is not a known one; a secret in a URL's path or query (`https://idp.example.com/mcp?api_key=`); a URL userinfo holding `/`, `?`, `#`, a quote, `,` or `;`, or white space other than a tab or a line break directly before the `@` — so a token followed by a space (`https:// @host/`) passes, and so does a tab inside the userinfo; a quote, `,` or `;` directly after the `://`; a tab or a line break inside the `://` itself (`https://user:@host/` — there is then no `://` to find); a user-only token whose scheme is broken by a tab, a line break or a space (`https://@host/` — it no longer reads as `http(s)`; with a password it is still refused); credentials with no `://` (`user:@host`); a user-only token on a scheme that is not `http(s)`, `ws(s)` or `+` one of them (`ssh://@host/`); anything that is neither a URL with a userinfo nor a token of a known format. One more way a value can show: a param value written as `${}` and rendered under `env` reads, to `doctor`'s `mcp-env`, as a variable reference: it prints `` as the name of a variable it expects when that has the shape of a name and is not set — a known token format inside the braces is refused before that, an unknown one is not. **Refused, though it is not a secret:** a URL on `http(s)`, `ws(s)` or `+` one of them with a user and no password (`https://acme@dev.azure.com/acme/Portal`); a userinfo that is not made only of plain references — a literal mixed with one (`postgres://acme:${DB_PASS}@db.example.com/app`; write `${DB_USER}:${DB_PASS}`) or a reference with a default (`https://${A:-x}@host/`); a URL with no path followed, with none of `/ ? #`, white space, a quote, `,` or `;` between, by text holding an `@` (`https://a.example.com&cc=ops@example.com`); a URL on any scheme but `file` written with an empty authority — extra `/`, `\` or white space after the `://` — when what follows then reads as a userinfo (`sqlite:///a:b@c.db`; on `http(s)`, `ws(s)` or `+` one of them an `@` alone is enough: `https:///ops@example.com`; in prose, `see :// a:b@c`); a URL with no path, query or fragment followed by a tab or by line breaks — a blank line between too — and then an `@` (`https://registry.npmjs.org`, then `@acme/pkg` on the next line; a markdown link or an unquoted CSS `url(…)` followed by a line that starts with `@`), since a tab, a line feed or a carriage return directly before an `@` is read past — on a scheme that is not `http(s)`, `ws(s)` or `+` one of them, only when the text before the `@` holds a `:`, such as a port; a standalone run of 52 lowercase letters and digits that holds a digit. The only way round a refusal is to keep the value out of a param: write it literally in the ingredient, or have the server read the environment. A literal the Forge author wrote in a server, with no placeholder in it, is **not** checked, at any schema: it is emitted as written, and `craftar import`'s scan stays the guard on what enters a Forge. +The check narrows the road; it does not close it. **Still passes, though it is a secret:** a token of an unknown format (outside `env` and `args` nothing looks for it; inside them it can only warn, and only when the whole value is letters, digits and `+ / = _ -`) — so `Bearer `, `KEY: ` and a `…;Password=…` connection string pass when the token's format is not a known one; a secret in a URL's path or query (`https://idp.example.com/mcp?api_key=`); a URL userinfo holding `/`, `?`, `#`, a quote, `,` or `;`, or white space other than a tab or a line break directly before the `@` — so a token followed by a space (`https:// @host/`) passes, and so does a tab inside the userinfo; a quote, `,` or `;` directly after the `://`; a user-only token whose scheme is broken by a space (`https://@host/` — it no longer reads as `http(s)`; with a password it is still refused; a tab or a line break inside the scheme or its `://` is read past since 0.32.1); credentials with no `://` (`user:@host`); a user-only token on a scheme that is not `http(s)`, `ws(s)` or `+` one of them (`ssh://@host/`); anything that is neither a URL with a userinfo nor a token of a known format. One more way a value can show: a param value written as `${}` and rendered under `env` reads, to `doctor`'s `mcp-env`, as a variable reference: it prints `` as the name of a variable it expects when that has the shape of a name and is not set — a known token format inside the braces is refused before that, an unknown one is not. **Refused, though it is not a secret:** a URL on `http(s)`, `ws(s)` or `+` one of them with a user and no password (`https://acme@dev.azure.com/acme/Portal`); a userinfo that is not made only of plain references — a literal mixed with one (`postgres://acme:${DB_PASS}@db.example.com/app`; write `${DB_USER}:${DB_PASS}`) or a reference with a default (`https://${A:-x}@host/`); a URL with no path followed, with none of `/ ? #`, white space, a quote, `,` or `;` between, by text holding an `@` (`https://a.example.com&cc=ops@example.com`); a URL on any scheme but `file` written with an empty authority — extra `/`, `\` or white space after the `://` — when what follows then reads as a userinfo (`sqlite:///a:b@c.db`; on `http(s)`, `ws(s)` or `+` one of them an `@` alone is enough: `https:///ops@example.com`; in prose, `see :// a:b@c`); a URL with no path, query or fragment followed by a tab or by line breaks — a blank line between too — and then an `@` (`https://registry.npmjs.org`, then `@acme/pkg` on the next line; a markdown link or an unquoted CSS `url(…)` followed by a line that starts with `@`), since a tab, a line feed or a carriage return directly before an `@` is read past — on a scheme that is not `http(s)`, `ws(s)` or `+` one of them, only when the text before the `@` holds a `:`, such as a port; a standalone run of 52 lowercase letters and digits that holds a digit. The only way round a refusal is to keep the value out of a param: write it literally in the ingredient, or have the server read the environment. A literal the Forge author wrote in a server, with no placeholder in it, is **not** checked, at any schema: it is emitted as written, and `craftar import`'s scan stays the guard on what enters a Forge. **Sections.** A body (its **body files**: the `file` of a rule, agent, command or steering — `rule.md` etc. by default; `SKILL.md` of a file-layout skill; every `.md`, `.txt`, `.json`, `.yaml` or `.yml` file of a dir-layout skill; the text files listed in `files` of a script or hook — a body file stays inside the ingredient directory; one declared outside it, or behind a symlinked directory, or spelled differently from the file on disk (letter case, on a case-insensitive file system), fails `sync`, `status`, `diff`, `explain` and `ls`, naming it) can hold blocks a profile or a workspace replaces. A file no target emits (a `notes.md` beside `rule.md`) is ignored — for sections and for `{{param}}` citations alike; a file a target emits but does not render as text is copied (see below). A block sits between two marker lines, and its content is the default: @@ -495,6 +495,13 @@ Later: rulesync bridge for Codex/Kimi/Cursor specifics; `craftar docs validate`; ## Upgrading +### to 0.32.1 + +- **A cache entry no clean registry row vouches for is printed by a label.** An entry an earlier release named after a URL with credentials keeps that name on disk, and once no registry row names it — right after a re-sync without the credentials — `craftar doctor` and `craftar cache prune` printed it, the user and the password with it. Now a cache entry's name is printed only when a registry row with a clean URL, or the checked workspace, names it; every other one — an orphan, and every entry when `CRAFTAR_NO_REGISTRY` is set or the registry cannot be read — reads `(a cache entry name — not shown)`: `doctor`'s `nothing names it` and `a fetch never completed` lines, and `cache prune`'s rows, its `--json` `path` (`forges/(a cache entry name — not shown)`, `forges/~removing-(a cache entry name — not shown)-`) and its warnings. Two such rows read the same; each keeps its kind, reason and size, and nothing on disk is renamed. An entry a registry row with credentials names keeps `(a URL with credentials — not shown)`, as since 0.28.1. Where there is no snapshot of the cache — `doctor`'s `cannot read the Forge cache: …` and `cache prune`'s `cannot read …/forges: …` — the name that follows `forges/` in the error is that label whoever names it, a key labelled `(a URL with credentials — not shown)` since 0.28.1 included. +- **More `forge:` spellings are refused as a URL with credentials**: a scheme followed by a single `/` (`https:/:@host/…`, or `https:/@host/…`), and a tab, a line feed or a carriage return inside the scheme or its `://` (`https://…`, `https://@…`) — read before as a path, and printed whole by `Forge not found at …`. They now get `… › forge holds credentials in the URL — remove them and let git authenticate`, in `craftar.yaml` and `craftar.local.yaml`, `init --forge` and its questions, `import --write-config` — which replaced such a value with the clone's path and exited 0, and now exits 1 and leaves it — a remote of a Forge clone and a template `source`; the exit code is 1 as before — and 1, where it was 0, when a directory of that name existed and loaded. A `ref:` spelled that way is named by the label and refused as an answer of the interactive `craftar init`, which also no longer offers a Forge recorded as a path whose spelling holds a credential. A `missing` or `error` row the registry recorded with such a spelling now reads `(a URL with credentials — not shown)` as its `forge.source`, with `forge.key: null`, in `craftar workspaces --json` and in the `craftar ui` table. A `forge:` that is a drive path holding `@` and a second `:` (`C:/x:y@z`) is refused too (fail-closed). +- **In a Forge at `schema: 4` or above, a server param whose rendered value holds a credential behind a tab or a line break inside the scheme or its `://` now fails the plan**, as one behind a plain `://` did: a workspace that synced with such a value stops syncing until the value leaves the param — a refusal, no byte written. +- No emitted byte, lock, registry entry or cache layout changes, and no `--json` key is added or removed. + ### to 0.32.0 - **A workspace whose `AGENTS.md` is above 32768 bytes now gets a warning on every plan.** `sync --check`, `status` and `diff --exit-code` fail only when `agents-md.maxBytes` is set explicitly; a writing `sync` still writes. @@ -545,7 +552,7 @@ Later: rulesync bridge for Codex/Kimi/Cursor specifics; `craftar docs validate`; ### to 0.28.1 - **A Forge URL with credentials that an earlier release recorded is no longer printed from the registry.** A release before `0.27.1` could record such a URL (see *to 0.27.1*). For a `missing` or an `error` row whose recorded URL holds credentials, `craftar workspaces --json` now gives `(a URL with credentials — not shown)` as `forge.source` and `null` as `forge.key`. `craftar cache prune` — its lines and, under `--json`, `kept[].path`, `removed[].path` and `warnings` — and the `cache` check of `craftar doctor` put the same label where the name of the cache entry such a row names would be: `path` then reads `forges/(a URL with credentials — not shown)`, `forges/(a URL with credentials — not shown)/trees/` or `forges/~removing-(a URL with credentials — not shown)-`, which is not a path, and two such entries read the same; a failure either command reports with a path inside such an entry names it the same way. No `--json` key is added, removed or renamed, and no type changes: `forge.key` could already be `null` (a path Forge that cannot be resolved); a `remote` row can now carry it too. A row that loads gives what `craftar.yaml` holds, as before. -- **A cache entry that no registered workspace names any more is still printed by its name.** Once every workspace that named the URL has synced again without the credentials, the registry no longer records it, and nothing in the cache says which entry was named after a refused URL. `craftar doctor` (` — nothing names it`, ` — a fetch never completed`) and `craftar cache prune` (the line of the entry it removes or keeps, of a tree of one it keeps, and a warning about it) then print that name, which holds the user and the password. With `CRAFTAR_NO_REGISTRY` set or a registry that cannot be read, `cache prune` names every entry that way, and `doctor` every one whose fetch never completed. Run `craftar cache prune` right after the re-sync, in a terminal rather than where the output is kept (a CI log, a file) — and rotate the credential either way. +- **A cache entry that no registered workspace names any more is still printed by its name** — until 0.32.1, which prints it by a label (see *to 0.32.1*). Once every workspace that named the URL has synced again without the credentials, the registry no longer records it, and nothing in the cache says which entry was named after a refused URL. `craftar doctor` (` — nothing names it`, ` — a fetch never completed`) and `craftar cache prune` (the line of the entry it removes or keeps, of a tree of one it keeps, and a warning about it) then print that name, which holds the user and the password. With `CRAFTAR_NO_REGISTRY` set or a registry that cannot be read, `cache prune` names every entry that way, and `doctor` every one whose fetch never completed. Run `craftar cache prune` right after the re-sync, in a terminal rather than where the output is kept (a CI log, a file) — and rotate the credential either way. - **A `forge:` that holds credentials behind a `\` in place of the `//` is refused** — `https:\\user:token@host\…`, or any run of `/` and `\` after the scheme that holds a `\`. It was read as a path, and `Forge not found at …` printed it whole. It is now refused wherever a URL with credentials is (`… › forge holds credentials in the URL — remove them and let git authenticate`): `craftar.yaml` and `craftar.local.yaml`, `init --forge` and its questions, `import --write-config`, a remote of a Forge clone. As with `://`, what counts is any user on `http(s)` and a `user:password` on another scheme, and `file:` is left alone. A drive path (`C:\…`) is read as before — a name there cannot hold the `:` of a `user:password` — and so is such a spelling with no user and no password. - **A `ref:` that holds a URL with credentials is not quoted.** The messages that name a ref — `ref "…" is ignored: the Forge is a path`, `ref "…" not found in `, `ref "…" is both a branch and a tag` (which then gives no `write refs/heads/…` hint) and `… the cached copy cannot resolve …` — read `ref (a URL with credentials — not shown)` for such a value, as the whole of it or inside it. The interactive `craftar init` says `Ref: (a URL with credentials — not shown) (from craftar.local.yaml)` for one it reads there, and refuses one typed as an answer (`the answer holds credentials in a URL — a ref is a branch, a tag or a full SHA`), where it offered it back as the next default. Every other ref is printed as before. - **No byte change**: no emitter changed, no lock changes and no workspace sees `update`. No exit code changes for a workspace whose `forge:` is still accepted. In one whose `forge:` is spelled with the `\` and holds credentials, the commands that load it failed before (`Forge not found at …`) and fail now with the refusal; `craftar import --write-config`, which replaced the value, now exits 1 with the Forge untouched, where it exited 0; `craftar cache prune` run there, or with `--workspace` naming it, keeps every whole entry (`unchecked`: the checked `craftar.yaml` does not read) where it pruned; and the `cache` check of `craftar doctor` there no longer reports an orphan — its line, or lines, end `(orphans not checked: the checked craftar.yaml does not read)`. diff --git a/package-lock.json b/package-lock.json index a591b48c..22183f40 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "craftar", - "version": "0.32.0", + "version": "0.32.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "craftar", - "version": "0.32.0", + "version": "0.32.1", "license": "MIT", "dependencies": { "commander": "^13.1.0", diff --git a/package.json b/package.json index 61245a3b..4578d1ae 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "craftar", - "version": "0.32.0", + "version": "0.32.1", "description": "Craft, sync and convert AI-coding workspace harnesses across clients and tools.", "license": "MIT", "type": "module", diff --git a/src/cli.ts b/src/cli.ts index 8c1f80a6..4bbc1a22 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -69,7 +69,7 @@ process.stdout.on("error", (e: NodeJS.ErrnoException) => { if (e.code === "EPIPE const DIFF_EXIT_CODE_FETCH_MODE: FetchMode = "sync"; const program = new Command(); -program.name("craftar").description("Craft, sync and convert AI-coding workspace harnesses.").version("0.32.0"); +program.name("craftar").description("Craft, sync and convert AI-coding workspace harnesses.").version("0.32.1"); /* ---------------------------------------------------------------- import */ /** The words of one output-path gate (`gateOutsideForge`), declared before the commands that run at load: what is written, by which command, and its two refusals' endings. */ diff --git a/src/core/cache.ts b/src/core/cache.ts index eebd903f..57f3da76 100644 --- a/src/core/cache.ts +++ b/src/core/cache.ts @@ -1,7 +1,7 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { LockBusyError, type LockTiming } from "./home-lock.js"; -import { credentialKeys, hideKeys, namedCacheKeys, readRegistry } from "./registry.js"; +import { credentialKeys, hideCacheKeys, hideCacheSegment, hideKeys, namedCacheKeys, readRegistry, vouchedKeys } from "./registry.js"; import type { Registry } from "../schema/index.js"; import { cacheDir, @@ -11,6 +11,7 @@ import { pruneTrees, removeEntry, removeLeftover, + REMOVING_PREFIX, withPruneLock, type CacheSnapshot, type GitRunner, @@ -72,6 +73,8 @@ export interface PruneOptions extends LockTiming { rename?: (from: string, to: string) => Promise; /** Test hook: awaited after each `refresh()` of the prune lock, before the step it precedes. */ beforeStep?: () => Promise; + /** Test hook: a replacement for the real `inspectCache` (spec 39 §5.1). */ + inspectCache?: typeof inspectCache; } export async function pruneCache(home: string, opts: PruneOptions): Promise { @@ -91,10 +94,19 @@ export async function pruneCache(home: string, opts: PruneOptions): Promise Promise; bytes: number }) => { + // Reset the state in case of retry (spec 39 §5.1: an entry unvouched at the snapshot stays labelled for the whole run). + snapshotBuilt = false; + hide.length = 0; + unvouched.length = 0; + const start = Date.now(); // Step 2: Determine naming info (§4.2 step 2) — under the lock so a waiting prune sees the state @@ -132,8 +144,9 @@ export async function pruneCache(home: string, opts: PruneOptions): Promise`. + for (const r of snapshot.removing) { + const name = path.basename(r.dir); + if (name.startsWith(REMOVING_PREFIX)) { + const rest = name.slice(REMOVING_PREFIX.length); + const lastDash = rest.lastIndexOf("-"); + if (lastDash !== -1) { + const maybeKey = rest.slice(0, lastDash); + if (!vouched.has(maybeKey) && !hide.includes(maybeKey)) unvouched.push(maybeKey); + } + } + } + snapshotBuilt = true; // Mark that the snapshot was built for guarded's hide logic. + const removed: PruneRemoved[] = []; const kept: PruneKept[] = []; const warnings: string[] = []; @@ -303,10 +335,14 @@ export async function pruneCache(home: string, opts: PruneOptions): Promise sum + r.bytes, 0); // The report names such an entry by the fixed label, in every path and warning. - const shown = (rows: T[]): T[] => (hide.length === 0 ? rows : rows.map((r) => ({ ...r, path: hideKeys(r.path, hide) }))); + // hideCacheKeys labels both credential and unvouched keys by merged ranges of the original text (spec 39 review rounds 2-3). + // An entry unvouched at the snapshot stays labelled for the whole run (fail closed). + const hidePath = (text: string): string => hideCacheKeys(text, hide, unvouched); + const shownRemoved = removed.map((r) => ({ ...r, path: hidePath(r.path) })); + const shownKept = kept.map((r) => ({ ...r, path: hidePath(r.path) })); return { - report: { home, dryRun: opts.dryRun, removed: shown(removed), kept: shown(kept), freedBytes, warnings: warnings.map((w) => hideKeys(w, hide)) }, + report: { home, dryRun: opts.dryRun, removed: shownRemoved, kept: shownKept, freedBytes, warnings: warnings.map(hidePath) }, header, }; }; @@ -317,9 +353,23 @@ export async function pruneCache(home: string, opts: PruneOptions): Promise` — while the inserted + // label holds no such run and starts with `(` and holds spaces, so hideKeys can match neither + // inside nor across it. + // After the snapshot: use hideCacheKeys, one call over the original text (spec 39 review rounds 2-3). + let msg = e.message; + if (!snapshotBuilt) { + msg = hideCacheSegment(msg, home); + msg = hideKeys(msg, hide); + } else { + msg = hideCacheKeys(msg, hide, unvouched); + } + if (msg === e.message) throw e; // A new error on purpose — no `cause`, no copied `code`: the original carries the name. - throw new Error(hideKeys(e.message, hide)); + throw new Error(msg); } }; diff --git a/src/core/doctor.ts b/src/core/doctor.ts index e0ebe8aa..c45bc7fd 100644 --- a/src/core/doctor.ts +++ b/src/core/doctor.ts @@ -4,8 +4,8 @@ import { ENV_NAME, type Lock, type Registry } from "../schema/index.js"; import { written } from "./capabilities.js"; import { exists } from "./forge.js"; import { renderServer, rendersServers } from "./mcp-server.js"; -import { credentialKeys, hideKeys, isMissing, isRegistered, namedCacheKeys, readRegistry, registryFile, rowStatus } from "./registry.js"; -import { FULL_SHA, cacheKey, classifyForge, inspectCache, type CacheSnapshot } from "./remote.js"; +import { credentialKeys, hideCacheSegment, hideKeys, isMissing, isRegistered, namedCacheKeys, readRegistry, registryFile, rowStatus, vouchedKeys } from "./registry.js"; +import { FULL_SHA, cacheKey, classifyForge, CREDENTIAL_HIDDEN, inspectCache, UNVOUCHED_HIDDEN, type CacheSnapshot } from "./remote.js"; import { dockerVersion, dockerFinding, dockerRunner as defaultDockerRunner, fragmentPaths, type DockerRunner } from "./services.js"; import { paramsFor } from "./resolve.js"; import { SchemaError, errorText } from "./schema-fault.js"; @@ -64,6 +64,8 @@ export interface DoctorOptions { dotnetVersion?: () => Promise; /** Test seam: injected runner for `docker compose version --short` (spec 31 §4.7). */ dockerRunner?: DockerRunner; + /** Test seam: a replacement for the real `inspectCache` (spec 39 §5.1). */ + inspectCache?: typeof inspectCache; } async function defaultGitVersion(): Promise { @@ -95,8 +97,9 @@ export async function runDoctor(opts: DoctorOptions): Promise { // which `git` and `cache` need even when the Forge does not load (§5.1). let snapshot: CacheSnapshot | null = null; let snapshotError: string | null = null; + const doInspect = opts.inspectCache ?? inspectCache; try { - snapshot = await inspectCache(opts.home); + snapshot = await doInspect(opts.home); } catch (e) { snapshotError = oneLine(e); } @@ -161,7 +164,13 @@ export async function runDoctor(opts: DoctorOptions): Promise { // One cause, one line (§13 item 12): a home that is not a directory is `home`'s error. m("cache", "ok", "not checked ($CRAFTAR_HOME is not a directory)"); } else if (snapshot === null) { - m("cache", "warn", `cannot read the Forge cache: ${hideKeys(snapshotError ?? "", reg === null ? [] : credentialKeys(reg))}`, `fix or move $CRAFTAR_HOME (${home})`); + // No snapshot: use hideCacheSegment, then hideKeys (spec 39 §5.1). + // Two passes are safe: hideCacheSegment replaces the whole segment after `forges/`, and a + // credential key is a cacheKey — only [a-z0-9._-], ending in `-<12 hex>` — while the inserted + // label holds no such run and starts with `(` and holds spaces, so hideKeys can match neither + // inside nor across it. + const msg = hideCacheSegment(snapshotError ?? "", opts.home); + m("cache", "warn", `cannot read the Forge cache: ${hideKeys(msg, reg === null ? [] : credentialKeys(reg))}`, `fix or move $CRAFTAR_HOME (${home})`); } else { const summary = `${snapshot.entries.length} entr${snapshot.entries.length === 1 ? "y" : "ies"}, ${MB(snapshot.bytes)}`; let skip: string | null = registryWhyNot; @@ -169,14 +178,21 @@ export async function runDoctor(opts: DoctorOptions): Promise { const findings: Check[] = []; // A key the registry records for a URL with credentials is never printed (the name holds them). const hide = reg === null ? [] : credentialKeys(reg); + // Compute the named map before both loops so both can use it (spec 39 §5.1). + const checked = skip === null && reg !== null && remote && root !== null ? { key: cacheKey(merged!.config.forge.trim()), path: root } : null; + const named = skip === null && reg !== null ? namedCacheKeys(reg, checked) : new Map(); + const vouched = vouchedKeys(named, reg); + // `a fetch never completed`: show key as-is only when vouched, as CREDENTIAL_HIDDEN when credentialKeys holds it, else UNVOUCHED_HIDDEN. for (const e of snapshot.entries) { - if (!e.fetched) findings.push({ id: "cache", scope: "machine", level: "warn", message: `${hideKeys(e.key, hide)} — a fetch never completed`, fix: CACHE_FIX }); + if (!e.fetched) { + const label = vouched.has(e.key) ? e.key : hide.includes(e.key) ? CREDENTIAL_HIDDEN : UNVOUCHED_HIDDEN; + findings.push({ id: "cache", scope: "machine", level: "warn", message: `${label} — a fetch never completed`, fix: CACHE_FIX }); + } } + // `nothing names it`: always UNVOUCHED_HIDDEN (such a key is never vouched). if (skip === null && reg !== null) { - const checked = remote && root !== null ? { key: cacheKey(merged!.config.forge.trim()), path: root } : null; - const named = namedCacheKeys(reg, checked); for (const e of snapshot.entries) - if (e.fetched && !named.has(e.key)) findings.push({ id: "cache", scope: "machine", level: "warn", message: `${e.key} — nothing names it`, fix: CACHE_FIX }); + if (e.fetched && !named.has(e.key)) findings.push({ id: "cache", scope: "machine", level: "warn", message: `${UNVOUCHED_HIDDEN} — nothing names it`, fix: CACHE_FIX }); } // A skipped orphan part is said on every cache line, findings included (§4.3, §13 item 6). const why = skip === null ? "" : ` (orphans not checked: ${skip})`; diff --git a/src/core/init-flow.ts b/src/core/init-flow.ts index d842cc0d..f3a0399a 100644 --- a/src/core/init-flow.ts +++ b/src/core/init-flow.ts @@ -51,6 +51,9 @@ function offerableForge( exists: (dir: string) => boolean, ): OfferedForge | null { if (entry.forge.kind === "path") { + // Spec 39 §7, §14 item 8: a gap-2 spelling that once loaded as a path is recorded with a real + // path that holds the credential. + if (credentialFault(entry.forge.source)) return null; if (entry.forge.key === null) return null; if (!exists(entry.forge.key)) return null; return { forge: entry.forge.key, ref: null }; diff --git a/src/core/registry.ts b/src/core/registry.ts index 146ba022..f3564f6c 100644 --- a/src/core/registry.ts +++ b/src/core/registry.ts @@ -4,7 +4,7 @@ import { REGISTRY_SCHEMAS, RegistrySchema, type Lock, type Registry, type Regist import { SchemaError, shownSchema } from "./schema-fault.js"; import { exists } from "./forge.js"; import { withLock, type LockTiming } from "./home-lock.js"; -import { cacheKey, credentialFault, CREDENTIAL_HIDDEN } from "./remote.js"; +import { cacheKey, credentialFault, CREDENTIAL_HIDDEN, UNVOUCHED_HIDDEN } from "./remote.js"; import { loadWorkspace, plan, readLock, status, unsetDeclared, unsetSummary, WORKSPACE_FILE, type FileStatus, type Plan, type Workspace } from "./sync.js"; /* @@ -165,11 +165,128 @@ export function credentialKeys(reg: Registry): string[] { /** `text` with every one of `keys` replaced by the fixed label: what `cache prune` and `doctor` print of a cache entry. */ export function hideKeys(text: string, keys: readonly string[]): string { - let out = text; - for (const k of keys) out = out.split(k).join(CREDENTIAL_HIDDEN); + return hideCacheKeys(text, keys, []); +} + +/** + * The vouched keys: the keys of `named` minus `credentialKeys(reg)`. Empty when `reg` is null or `named` + * is empty (spec 39 §5.1). A vouched key is printed as-is; an unvouched key gets `UNVOUCHED_HIDDEN`. + */ +export function vouchedKeys(named: ReadonlyMap, reg: Registry | null): Set { + if (reg === null || named.size === 0) return new Set(); + const flagged = new Set(credentialKeys(reg)); + const vouched = new Set(); + for (const k of named.keys()) if (!flagged.has(k)) vouched.add(k); + return vouched; +} + +/** `text` with every one of `keys` replaced by `UNVOUCHED_HIDDEN`: what `cache prune` and `doctor` print for an unvouched key. */ +export function hideUnvouched(text: string, keys: readonly string[]): string { + return hideCacheKeys(text, [], keys); +} + +/** + * Replace every occurrence of every key in `credential` with `CREDENTIAL_HIDDEN` and every key in + * `unvouched` with `UNVOUCHED_HIDDEN`. One label per merged range of the original text: + * + * - A prefix key (review round 2): `abc` and `abcdef` starting at the same position — the longer + * key wins (it's the more specific match); if they start at different positions, each gets its + * own label. + * - A key that starts before another and ends inside it (review round 3): `removing-github` starts + * before `github.com-u-tok-abcdef012345` and ends inside it — merge them, one label. + * + * Credential wins: a merged range is labelled `CREDENTIAL_HIDDEN` when any key covering any part of + * it is in `credential`. Empty keys are ignored. Ranges are found on the ORIGINAL text, so nothing + * inside an inserted label is ever matched. + */ +export function hideCacheKeys(text: string, credential: readonly string[], unvouched: readonly string[]): string { + // Collect non-empty keys; credential wins when a key is in both lists + const credSet = new Set(credential.filter((k) => k !== "")); + const unvSet = new Set(unvouched.filter((k) => k !== "")); + const all: string[] = []; + for (const k of credSet) all.push(k); + for (const k of unvSet) if (!credSet.has(k)) all.push(k); + if (all.length === 0) return text; + + // Find every occurrence of every key: [start, end), credential flag + // When multiple keys start at the same position, keep only the longest (most specific match) + const byStart = new Map(); + for (const key of all) { + const isCred = credSet.has(key); + let pos = 0; + while (true) { + const idx = text.indexOf(key, pos); + if (idx === -1) break; + const existing = byStart.get(idx); + const keyEnd = idx + key.length; + if (existing === undefined || keyEnd > existing.end) { + // No existing range at this start, or this key is longer: take it (longer key wins completely) + // Deliberate: at the same start the longer key's label is used, even over a shorter credential key + // it covers whole — nothing of either is printed (spec 39 review round 4). + byStart.set(idx, { end: keyEnd, credential: isCred }); + } else if (keyEnd === existing.end && isCred && !existing.credential) { + // Same length but this one is a credential and existing is not: mark it + existing.credential = true; + } + pos = idx + 1; // advance by 1 so overlapping occurrences of the same key are found + } + } + if (byStart.size === 0) return text; + + // Convert to sorted array + const ranges: { start: number; end: number; credential: boolean }[] = []; + for (const [start, { end, credential }] of byStart) { + ranges.push({ start, end, credential }); + } + ranges.sort((a, b) => a.start - b.start); + + // Merge every range that overlaps the current one (next.start < cur.end) + // Touching ranges (next.start === cur.end) are NOT merged, so adjacent keys keep a label each + const merged: { start: number; end: number; credential: boolean }[] = []; + let cur = { ...ranges[0] }; + for (let i = 1; i < ranges.length; i++) { + const next = ranges[i]; + if (next.start < cur.end) { + // Overlaps: extend current range, credential wins + cur.end = Math.max(cur.end, next.end); + cur.credential = cur.credential || next.credential; + } else { + // No overlap: finalize current, start new + merged.push(cur); + cur = { ...next }; + } + } + merged.push(cur); + + // Build output left to right: text between ranges unchanged, each merged range replaced by label + let out = ""; + let prev = 0; + for (const r of merged) { + out += text.slice(prev, r.start); + out += r.credential ? CREDENTIAL_HIDDEN : UNVOUCHED_HIDDEN; + prev = r.end; + } + out += text.slice(prev); return out; } +/** + * `text` with every path segment that follows `/forges/` or `\forges\` + * (either separator after `forges`) up to the next `/`, `\`, white space or quote replaced by + * `UNVOUCHED_HIDDEN`. The no-snapshot rule, spec 39 §5.1 — vouched or not. + */ +export function hideCacheSegment(text: string, home: string): string { + // Strip trailing separators so the root `/` becomes empty and the pattern is `[/\\]forges[/\\]` + const resolved = path.resolve(home).replace(/[/\\]+$/, ""); + // Escape special regex characters in the home path + const escaped = resolved.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + // Match: /forges/ or \forges\ followed by the segment (up to / \ space or quote) + // Capture the prefix (including the separator after forges) in group 1 + const pattern = new RegExp(`(${escaped}[/\\\\]forges[/\\\\])([^/\\\\\\s"']+)`, "g"); + // Use a function replacement to avoid $&, $1, etc. in the home path being interpreted + return text.replace(pattern, (_, prefix: string) => prefix + UNVOUCHED_HIDDEN); +} + /** Whether `dir` has a registry entry — by real path, without case on Windows, as `register` keys it (spec 24 §5.1). */ export async function isRegistered(reg: Registry, dir: string): Promise { const target = await realOrResolved(dir); diff --git a/src/core/remote.ts b/src/core/remote.ts index 62bf015e..9ca4f508 100644 --- a/src/core/remote.ts +++ b/src/core/remote.ts @@ -17,12 +17,18 @@ const SCHEME_URL = /^[A-Za-z][A-Za-z0-9+.-]*:\/\//; /** The SCP-like form `@:`; without a user, `host:path` stays a path (a drive letter looks the same). */ const SCP_URL = /^[^\s/\\:@]+@[^\s/\\:]+:/; -/** A scheme and the run of `/` and `\` after it, whatever the run is (`credentialFault` reads it when it is not `//`). */ +/** A scheme and the run of `/` and `\` after it; `credentialFault` reads any run that is not `//` (spec 39 §4.2 A). */ const BACKSLASH_URL = /^([A-Za-z][A-Za-z0-9+.-]*):([/\\]+)/; /** What is printed in place of a value `credentialFault` flags, where one written or recorded earlier would be shown. */ export const CREDENTIAL_HIDDEN = "(a URL with credentials — not shown)"; +/** + * What is printed in place of a cache key no clean registry row vouches for — an orphan, every entry + * when the registry is off or unreadable. Spec 39 §3. + */ +export const UNVOUCHED_HIDDEN = "(a cache entry name — not shown)"; + /** A `forge:` value is a git URL (a remote Forge) or a path resolved from the workspace root. */ export function classifyForge(value: string): "url" | "path" { const v = value.trim(); @@ -46,17 +52,25 @@ const REFERENCE_ONLY = new RegExp(`^(\\$\\{${ENV_NAME}\\}|:)*$`); * the remedy it prescribes (a userinfo made only of `${NAME}` references). */ export function urlCredentialIn(text: string): boolean { + // A run of tab / LF / CR among the `:` and the two `/` of a scheme separator is removed, so the + // separator is found (spec 39 §5.2). + const leaf = text.replace(/:[\t\n\r]*\/[\t\n\r]*\//g, "://"); // Step 1: For every occurrence of `://` in the leaf (occurrences are independent; none hides another). let pos = 0; while (true) { - const sep = text.indexOf("://", pos); + const sep = leaf.indexOf("://", pos); if (sep === -1) break; pos = sep + 3; // Move past this `://` for the next iteration // Step 1: The scheme is the run of [A-Za-z0-9+.-] directly before it; it may be empty. let schemeStart = sep; - while (schemeStart > 0 && /[A-Za-z0-9+.-]/.test(text[schemeStart - 1])) schemeStart--; - const scheme = text.slice(schemeStart, sep).toLowerCase(); + while (schemeStart > 0 && /[A-Za-z0-9+.-]/.test(leaf[schemeStart - 1])) schemeStart--; + const scheme = leaf.slice(schemeStart, sep).toLowerCase(); + + // Spec 39 §5.2: also walk back including tab / LF / CR, then remove them and lower-case. + let schemeLooseStart = sep; + while (schemeLooseStart > 0 && /[A-Za-z0-9+.\-\t\n\r]/.test(leaf[schemeLooseStart - 1])) schemeLooseStart--; + const schemeLoose = leaf.slice(schemeLooseStart, sep).replace(/[\t\n\r]/g, "").toLowerCase(); // Step 2: The authority is the text after it, up to the first of `/`, `?`, `#`, white space (\s), `"`, `'`, `,`, `;` or the end. // An empty authority is not the end of it: curl takes up to three slashes after the scheme and reads what @@ -64,7 +78,7 @@ export function urlCredentialIn(text: string): boolean { // but `file` (where an empty authority is the norm and the rest is a path) is read past the leading // separators. White space among them is read past too: a URL parser drops a tab or a newline, // percent-encodes a space, and still reads the userinfo that follows. - const rest = text.slice(sep + 3); + const rest = leaf.slice(sep + 3); const afterScheme = scheme === "file" ? rest : rest.replace(/^[/\\\s]+/, ""); // White space ends the authority — except a run of tab, line feed and carriage return directly before an // `@`: a URL parser deletes those three from a URL, so `https://@host/` reaches the host with the @@ -101,9 +115,11 @@ export function urlCredentialIn(text: string): boolean { // Step 5: When the scheme is http, https, ws or wss, or ends in + followed by one of them, // any other non-empty userinfo is a finding; for any other scheme, the empty one included, - // only a userinfo holding `:` is. - const isHttpWs = HTTP_WS_SCHEMES.includes(scheme) || - HTTP_WS_SCHEMES.some((s) => scheme.endsWith("+" + s)); + // only a userinfo holding `:` is. Spec 39 §5.2: isHttpWs is true when EITHER reading + // (scheme or schemeLoose) matches. + const isHttpWs = + HTTP_WS_SCHEMES.includes(scheme) || HTTP_WS_SCHEMES.some((s) => scheme.endsWith("+" + s)) || + HTTP_WS_SCHEMES.includes(schemeLoose) || HTTP_WS_SCHEMES.some((s) => schemeLoose.endsWith("+" + s)); if (isHttpWs) { // Any non-empty userinfo (not made of references only) is a finding @@ -122,21 +138,23 @@ export function urlCredentialIn(text: string): boolean { * password never reaches a message that prints the value (spec 13 §6.1, §14 item 11). An empty * authority (`https:///…`) is read past, for every scheme but `file`: what follows the extra * separators is the authority some clients use — curl past up to three slashes, a WHATWG parser - * past a `\` too. A scheme followed by separators that are not `//` and hold a `\` (`https:\\…`, - * `https:/\…`) is read the same way: a WHATWG parser takes each `\` for a `/`, while + * past a `\` too. A scheme followed by any run of `/` and `\` that is not `//` (`https:/…`, + * `https:\\…`, `https:/\…`) is read the same way: a WHATWG parser takes each `\` for a `/`, while * `classifyForge` reads the value as a path — and a path is printed. */ export function credentialFault(raw: string): boolean { - const value = raw.trim(); + // A URL parser removes tab, line feed and carriage return before it reads, so `https://…` + // and `https://…` reach a host with their userinfo — spec 39 §4.2 B. + const value = raw.replace(/[\t\n\r]/g, "").trim(); const head = value.split(/[/\\]/, 1)[0]; if (/^[^@]*:[^@]*@/.test(head) && !SCHEME_URL.test(value)) return true; const scheme = SCHEME_URL.exec(value); if (!scheme) { - // No `://`: only a separator run holding a backslash is read as one. The authority ends at the - // first `/` or `\`; the rules are the ones below, by the scheme's name. A drive path (`C:\dir`) - // has no `@` there. + // No `://`: any run of `/` and `\` after the scheme that is not `//` has its authority read by + // the same rules (spec 39 §4.2 A). The authority ends at the first `/` or `\`; a drive path + // (`C:\dir`) has no `@` there. SCHEME_URL already takes a run starting with `//`. const loose = BACKSLASH_URL.exec(value); - if (!loose || !loose[2].includes("\\") || /^file$/i.test(loose[1])) return false; + if (!loose || /^file$/i.test(loose[1])) return false; const tail = value.slice(loose[0].length); const looseAuthority = tail.slice(0, tail.search(/[/\\]|$/)); const looseAt = looseAuthority.lastIndexOf("@"); diff --git a/test/cache-prune-contention.test.ts b/test/cache-prune-contention.test.ts index a5843974..350c7187 100644 --- a/test/cache-prune-contention.test.ts +++ b/test/cache-prune-contention.test.ts @@ -2,7 +2,7 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { pruneCache, type PruneOptions } from "../src/core/cache.js"; -import { cacheKey, ensureTree } from "../src/core/remote.js"; +import { cacheKey, ensureTree, UNVOUCHED_HIDDEN } from "../src/core/remote.js"; import { tmpDir, profile, recipe, rule, writeFiles } from "./helpers/forge.js"; import { remoteForge } from "./helpers/remote.js"; @@ -48,8 +48,8 @@ describe("a crash mid-removal (§4.3)", () => { expect(await ls(c.forges)).toEqual([c.key, `~removing-${c.key}-99999`, "prune.lock"].sort()); const dry = (await pruneCache(c.home, { ...RUN, dryRun: true })).report; expect(dry.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([ - [`forges/${c.key}`, "entry", "incomplete"], - [`forges/~removing-${c.key}-99999`, "leftover", "interrupted"], + [`forges/${UNVOUCHED_HIDDEN}`, "entry", "incomplete"], + [`forges/~removing-${UNVOUCHED_HIDDEN}-99999`, "leftover", "interrupted"], ]); }); @@ -59,8 +59,8 @@ describe("a crash mid-removal (§4.3)", () => { await fs.utimes(path.join(c.forges, "prune.lock"), old, old); const { report } = await pruneCache(c.home, { ...RUN, staleMs: 1_000 }); expect(report.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([ - [`forges/${c.key}`, "entry", "incomplete"], - [`forges/~removing-${c.key}-99999`, "leftover", "interrupted"], + [`forges/${UNVOUCHED_HIDDEN}`, "entry", "incomplete"], + [`forges/~removing-${UNVOUCHED_HIDDEN}-99999`, "leftover", "interrupted"], ]); expect([report.kept, report.warnings]).toEqual([[], []]); expect(await ls(c.forges)).toEqual([]); @@ -139,8 +139,8 @@ describe("a file held open, leftovers, and a key shaped like a removal directory const { report } = await pruneCache(s.home, { ...RUN, rename }); expect([report.removed, report.kept, report.warnings]).toEqual([ [], - [{ path: `forges/${s.key}`, reason: "held-open", namedBy: [] }], - [`${s.key} is held open (EBUSY) — kept`], + [{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "held-open", namedBy: [] }], + [`${UNVOUCHED_HIDDEN} is held open (EBUSY) — kept`], ]); expect(await ls(s.entry)).toEqual(["fetched", "repo.git", "trees"]); expect(await ls(s.forges)).toEqual([s.key]); @@ -151,8 +151,8 @@ describe("a file held open, leftovers, and a key shaped like a removal directory await fs.writeFile(path.join(s.home, "registry.json"), JSON.stringify({ schema: 1, workspaces: [] })); await writeFiles(s.forges, { "~removing-example.com-old-0a1b2c3d4e5f-4242/repo.git/HEAD": "12345" }); const { report } = await pruneCache(s.home, { ...RUN, registryOff: true }); - expect(report.removed).toEqual([{ path: "forges/~removing-example.com-old-0a1b2c3d4e5f-4242", kind: "leftover", reason: "interrupted", bytes: 5 }]); - expect(report.kept).toEqual([{ path: `forges/${s.key}`, reason: "unchecked", namedBy: [] }]); + expect(report.removed).toEqual([{ path: `forges/~removing-${UNVOUCHED_HIDDEN}-4242`, kind: "leftover", reason: "interrupted", bytes: 5 }]); + expect(report.kept).toEqual([{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "unchecked", namedBy: [] }]); expect(report.freedBytes).toBe(5); expect(await ls(s.forges)).toEqual([s.key]); }); diff --git a/test/cache-prune.test.ts b/test/cache-prune.test.ts index cd449dc1..294039a3 100644 --- a/test/cache-prune.test.ts +++ b/test/cache-prune.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { pruneCache, type PruneOptions } from "../src/core/cache.js"; import { prune as pruneRegistry, register } from "../src/core/registry.js"; -import { cacheKey, ensureTree, inspectCache } from "../src/core/remote.js"; +import { cacheKey, ensureTree, inspectCache, UNVOUCHED_HIDDEN } from "../src/core/remote.js"; import { apply, loadWorkspace, plan, readLock, status } from "../src/core/sync.js"; import { makeWorkspace, profile, recipe, rule, tmpDir, writeFiles } from "./helpers/forge.js"; import { remoteForge } from "./helpers/remote.js"; @@ -54,7 +54,7 @@ describe("whole entries (§4.2 steps 2–4)", () => { expect(report).toEqual({ home: s.home, dryRun: false, - removed: [{ path: `forges/${s.key}`, kind: "entry", reason: "orphan", bytes }], + removed: [{ path: `forges/${UNVOUCHED_HIDDEN}`, kind: "entry", reason: "orphan", bytes }], kept: [], freedBytes: bytes, warnings: [], @@ -78,7 +78,7 @@ describe("whole entries (§4.2 steps 2–4)", () => { await fs.rm(dir, { recursive: true, force: true }); expect((await pruneCache(s.home, RUN)).report.kept).toEqual([{ path: `forges/${s.key}`, reason: "named", namedBy: [dir] }]); expect(await pruneRegistry(s.home)).toEqual([dir]); - expect((await pruneCache(s.home, RUN)).report.removed.map((x) => [x.path, x.reason])).toEqual([[`forges/${s.key}`, "orphan"]]); + expect((await pruneCache(s.home, RUN)).report.removed.map((x) => [x.path, x.reason])).toEqual([[`forges/${UNVOUCHED_HIDDEN}`, "orphan"]]); }); it("named only by the current, unregistered workspace: kept; from another directory: removed", async () => { @@ -86,7 +86,7 @@ describe("whole entries (§4.2 steps 2–4)", () => { const dir = await s.ws("acme-api"); await loadWorkspace(dir, { home: s.home }); expect((await pruneCache(s.home, { ...RUN, workspace: dir })).report.kept).toEqual([{ path: `forges/${s.key}`, reason: "named", namedBy: [dir] }]); - expect((await pruneCache(s.home, RUN)).report.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([[`forges/${s.key}`, "entry", "orphan"]]); + expect((await pruneCache(s.home, RUN)).report.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([[`forges/${UNVOUCHED_HIDDEN}`, "entry", "orphan"]]); }); it("registry off: no whole entry removed, every one unchecked, the warning; old trees still go", async () => { @@ -94,8 +94,8 @@ describe("whole entries (§4.2 steps 2–4)", () => { const t = await ensureTree(s.r.url, null, { home: s.home }); await fs.utimes(`${t.dir}.used`, ago(15 * DAY), ago(15 * DAY)); const { report } = await pruneCache(s.home, { ...RUN, registryOff: true }); - expect(report.kept).toEqual([{ path: `forges/${s.key}`, reason: "unchecked", namedBy: [] }]); - expect(report.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([[`forges/${s.key}/trees/${t.commit}`, "tree", "unused"]]); + expect(report.kept).toEqual([{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "unchecked", namedBy: [] }]); + expect(report.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([[`forges/${UNVOUCHED_HIDDEN}/trees/${t.commit}`, "tree", "unused"]]); expect(report.warnings).toEqual(["whole entries kept: CRAFTAR_NO_REGISTRY is set — nothing can tell which ones are used"]); expect(await ls(path.join(s.entry, "trees"))).toEqual([]); }); @@ -107,7 +107,7 @@ describe("whole entries (§4.2 steps 2–4)", () => { const { report } = await pruneCache(s.home, RUN); expect([report.removed, report.kept, report.warnings]).toEqual([ [], - [{ path: `forges/${s.key}`, reason: "unchecked", namedBy: [] }], + [{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "unchecked", namedBy: [] }], ["whole entries kept: the registry cannot be read — nothing can tell which ones are used"], ]); }); @@ -120,7 +120,7 @@ describe("whole entries (§4.2 steps 2–4)", () => { const { report } = await pruneCache(s.home, { ...RUN, workspace: dir }); expect([report.removed, report.kept, report.warnings]).toEqual([ [], - [{ path: `forges/${s.key}`, reason: "unchecked", namedBy: [] }], + [{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "unchecked", namedBy: [] }], ["whole entries kept: the checked craftar.yaml does not read — nothing can tell which ones are used"], ]); }); @@ -154,7 +154,7 @@ describe("whole entries (§4.2 steps 2–4)", () => { const dir = await s.ws("acme-api"); await s.syncAndRegister(dir); await fs.rm(path.join(s.entry, "fetched")); - expect((await pruneCache(s.home, { ...RUN, workspace: dir, registryOff: true })).report.kept).toEqual([{ path: `forges/${s.key}`, reason: "unchecked", namedBy: [] }]); + expect((await pruneCache(s.home, { ...RUN, workspace: dir, registryOff: true })).report.kept).toEqual([{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "unchecked", namedBy: [] }]); }); it("no forges/ directory: nothing to prune, and forges/ is not created", async () => { @@ -203,10 +203,10 @@ describe("--dry-run (§6 case 7)", () => { home: s.home, dryRun: true, removed: [ - { path: `forges/${s.key}/trees/${t.commit}`, kind: "tree", reason: "unused", bytes: expect.any(Number) }, - { path: "forges/~removing-example.com-old-0a1b2c3d4e5f-4242", kind: "leftover", reason: "interrupted", bytes: 3 }, + { path: `forges/${UNVOUCHED_HIDDEN}/trees/${t.commit}`, kind: "tree", reason: "unused", bytes: expect.any(Number) }, + { path: `forges/~removing-${UNVOUCHED_HIDDEN}-4242`, kind: "leftover", reason: "interrupted", bytes: 3 }, ], - kept: [{ path: `forges/${s.key}`, reason: "unchecked", namedBy: [] }], + kept: [{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "unchecked", namedBy: [] }], freedBytes: dry.removed[0].bytes + 3, warnings: ["whole entries kept: CRAFTAR_NO_REGISTRY is set — nothing can tell which ones are used"], }); @@ -222,7 +222,7 @@ describe("--dry-run (§6 case 7)", () => { const s = await setup(); await ensureTree(s.r.url, null, { home: s.home }); const dry = (await pruneCache(s.home, { ...RUN, dryRun: true })).report; - expect(dry.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([[`forges/${s.key}`, "entry", "orphan"]]); + expect(dry.removed.map((x) => [x.path, x.kind, x.reason])).toEqual([[`forges/${UNVOUCHED_HIDDEN}`, "entry", "orphan"]]); expect(await ls(path.join(s.home, "forges"))).toEqual([s.key]); expect({ ...(await pruneCache(s.home, RUN)).report, dryRun: true }).toEqual(dry); }); @@ -268,13 +268,14 @@ describe("the report (§4.5) and what is never touched (§7)", () => { }); describe("the re-check under the entry lock (§4.3 step 1)", () => { - it("a workspace registered between the snapshot and the entry's lock: kept named, namedBy from the re-check", async () => { + // An entry unvouched at the snapshot stays labelled even when the re-check vouches it (fail closed, review R1). + it("a workspace registered between the snapshot and the entry's lock: kept named, namedBy from the re-check, path labelled", async () => { const s = await setup(); await ensureTree(s.r.url, null, { home: s.home }); const dir = await s.ws("acme-api"); let steps = 0; const { report } = await pruneCache(s.home, { ...RUN, beforeStep: async () => { if (steps++ === 0) await s.syncAndRegister(dir); } }); - expect([report.removed, report.kept, report.warnings]).toEqual([[], [{ path: `forges/${s.key}`, reason: "named", namedBy: [dir] }], []]); + expect([report.removed, report.kept, report.warnings]).toEqual([[], [{ path: `forges/${UNVOUCHED_HIDDEN}`, reason: "named", namedBy: [dir] }], []]); }); it("the registry stops reading at the re-check: every such entry kept unchecked, the warning once", async () => { diff --git a/test/cli.test.ts b/test/cli.test.ts index a396b524..7e5f0bac 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -9,7 +9,7 @@ import { git, remoteForge } from "./helpers/remote.js"; import { TSX_LOADER } from "./helpers/tsx-loader.js"; import { exists, listFiles } from "../src/core/forge.js"; import { UnifyPlanSchema } from "../src/schema/index.js"; -import { cacheKey, ensureTree } from "../src/core/remote.js"; +import { cacheKey, ensureTree, UNVOUCHED_HIDDEN } from "../src/core/remote.js"; const cleanups: Array<() => Promise> = []; afterEach(async () => { @@ -6117,7 +6117,8 @@ describe("cli — craftar cache prune (spec 26 §4)", () => { expect([out.code, out.stderr]).toEqual([0, ""]); const lines = out.stdout.split("\n"); expect(lines[0]).toMatch(new RegExp(`^craftar cache prune — ${s.forges.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")} \\(1 entry, \\d+\\.\\d MB\\)$`)); - expect(lines[1]).toMatch(new RegExp(`^ removed {6}${s.key} orphan: nothing names it \\d+\\.\\d MB$`)); + const UNVOUCHED = UNVOUCHED_HIDDEN.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + expect(lines[1]).toMatch(new RegExp(`^ removed {6}${UNVOUCHED} orphan: nothing names it \\d+\\.\\d MB$`)); expect(lines.slice(2)).toEqual([expect.stringMatching(/^freed \d+\.\d MB$/), ""]); expect(await fs.readdir(s.forges)).toEqual([]); }); @@ -6126,8 +6127,9 @@ describe("cli — craftar cache prune (spec 26 §4)", () => { const s = await setup(); const out = s.run(["--dry-run"]); expect(out.code).toBe(0); + const UNVOUCHED = UNVOUCHED_HIDDEN.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); expect(out.stdout.split("\n").slice(1)).toEqual([ - expect.stringMatching(new RegExp(`^ would remove ${s.key} orphan: nothing names it \\d+\\.\\d MB$`)), + expect.stringMatching(new RegExp(`^ would remove ${UNVOUCHED} orphan: nothing names it \\d+\\.\\d MB$`)), expect.stringMatching(/^would free \d+\.\d MB$/), "", ]); @@ -6143,7 +6145,7 @@ describe("cli — craftar cache prune (spec 26 §4)", () => { expect([report.home, report.dryRun, report.removed.map((x: { path: string; kind: string; reason: string }) => [x.path, x.kind, x.reason]), report.kept, report.warnings]).toEqual([ s.home, true, - [[`forges/${s.key}`, "entry", "orphan"]], + [[`forges/${UNVOUCHED_HIDDEN}`, "entry", "orphan"]], [], [], ]); @@ -6158,7 +6160,7 @@ describe("cli — craftar cache prune (spec 26 §4)", () => { const off = s.run([], { env: { CRAFTAR_NO_REGISTRY: "1" } }); expect(off.code).toBe(0); expect(off.stdout.split("\n").slice(1)).toEqual([ - ` kept ${s.key} not checked`, + ` kept ${UNVOUCHED_HIDDEN} not checked`, " warn whole entries kept: CRAFTAR_NO_REGISTRY is set — nothing can tell which ones are used", "freed 0.0 MB", "", diff --git a/test/credential-gaps-cli.test.ts b/test/credential-gaps-cli.test.ts new file mode 100644 index 00000000..62dcc634 --- /dev/null +++ b/test/credential-gaps-cli.test.ts @@ -0,0 +1,167 @@ +/** + * Spec 39 gap 2 — CLI tests for single-slash credential spellings (`https:/user:pw@...`). + * + * These spellings used to be read as paths and printed whole. Now `credentialFault` flags them, + * so every CLI command that loads a workspace refuses them with the credential message. + */ +import { promises as fs } from "node:fs"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { makeForge, profile, recipe, rule, scenario, tmpDir, writeFiles } from "./helpers/forge.js"; +import { runCli } from "./helpers/cli.js"; + +const cleanups: Array<() => Promise> = []; +afterEach(async () => { + while (cleanups.length) await cleanups.pop()!(); +}); + +// Fake credentials assembled at runtime so a grep never finds them as a literal. +const USER = ["fake", "user"].join(""); +const PW = ["NOTA", "SECRET"].join(""); +const CRED = `${USER}:${PW}@`; + +describe("spec 39 gap 2: a forge: spelled with one slash after the scheme", () => { + // Single-slash spellings — a scheme followed by one `/` instead of `//`. + const spellings = [ + `https:/${CRED}example.invalid/acme/forge.git`, + `https:/${USER}@example.invalid/acme/forge.git`, + `http:/${CRED}example.invalid/acme/forge.git`, + `ssh:/${CRED}example.invalid/acme/forge.git`, + ]; + + /** Set up a workspace with a craftar.yaml that holds a single-slash credential. */ + async function wsWithForge(forgeValue: string) { + const root = await tmpDir("craftar-gap2-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + const forge = path.join(root, "forge"); + const ws = path.join(root, "ws"); + await makeForge(forge, { + ingredients: [rule("a", "# A\n")], + recipes: [recipe("base", ["rule/a"])], + profiles: [profile("acme", ["base"])], + }); + await fs.mkdir(ws); + await fs.writeFile(path.join(ws, "craftar.yaml"), `forge: ${forgeValue}\nprofile: acme\n`); + const home = path.join(root, "home"); + await fs.mkdir(home); + return { root, forge, ws, home }; + } + + it("sync: exit 1, the credential message, nothing of the fake password printed", async () => { + for (const forge of spellings) { + const { ws, home } = await wsWithForge(forge); + const r = runCli(["sync", "--workspace", ws], { env: { CRAFTAR_HOME: home } }); + expect(r.code, forge.replace(PW, "")).toBe(1); + expect(r.stderr, forge.replace(PW, "")).toContain("craftar.yaml › forge holds credentials in the URL"); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW.toLowerCase()); + } + }); + + it("status: exit 1, the credential message, nothing of the fake password printed", async () => { + for (const forge of spellings) { + const { ws, home } = await wsWithForge(forge); + const r = runCli(["status", "--workspace", ws], { env: { CRAFTAR_HOME: home } }); + expect(r.code, forge.replace(PW, "")).toBe(1); + expect(r.stderr, forge.replace(PW, "")).toContain("craftar.yaml › forge holds credentials in the URL"); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW.toLowerCase()); + } + }); + + it("init --forge: exit 1, the credential message, nothing of the fake password printed", async () => { + for (const forge of spellings) { + const root = await tmpDir("craftar-gap2-init-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + const ws = path.join(root, "ws"); + await fs.mkdir(ws); + const home = path.join(root, "home"); + await fs.mkdir(home); + const r = runCli(["init", "--workspace", ws, "--forge", forge, "--profile", "acme"], { env: { CRAFTAR_HOME: home } }); + expect(r.code, forge.replace(PW, "")).toBe(1); + expect(r.stderr, forge.replace(PW, "")).toContain("--forge holds credentials in the URL"); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW.toLowerCase()); + } + }); + + it("import --write-config: exit 1, the credential message, craftar.yaml unchanged, nothing of the fake password printed", async () => { + for (const forge of spellings) { + const { ws, home, forge: forgeDir } = await wsWithForge(forge); + // Set up a minimal .claude/rules file so import has something to read + await writeFiles(ws, { ".claude/rules/a.md": "# A\n" }); + // Write an initial craftar.yaml with the credential URL + const beforePath = path.join(ws, "craftar.yaml"); + const beforeContent = `forge: ${forge}\nprofile: acme\n`; + await fs.writeFile(beforePath, beforeContent); + const r = runCli(["import", "--from", "claude-code", "--workspace", ws, "--forge", forgeDir, "--profile", "acme", "--write-config"], { + env: { CRAFTAR_HOME: home }, + }); + expect(r.code, forge.replace(PW, "")).toBe(1); + expect(r.stderr, forge.replace(PW, "")).toContain("craftar.yaml › forge holds credentials in the URL"); + // craftar.yaml is unchanged (the refusal wrote nothing). + const afterContent = await fs.readFile(beforePath, "utf8"); + expect(afterContent, "craftar.yaml was changed despite the refusal").toBe(beforeContent); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW); + expect(r.stdout + r.stderr, forge.replace(PW, "")).not.toContain(PW.toLowerCase()); + } + }); +}); + +describe("spec 39 gap 2: a forge: spelled with tab/LF/CR inside the scheme or its ://", () => { + // Tab/LF/CR spellings — control characters that a URL parser removes. + const spellings = [ + `https:\t//${CRED}example.invalid/acme/forge.git`, + `https:/\n/${USER}@example.invalid/acme/forge.git`, + `ht\ttps://${USER}@example.invalid/acme/forge.git`, + `https:\r//${CRED}example.invalid/acme/forge.git`, + ]; + + it("sync: exit 1, the credential message, nothing of the fake password printed", async () => { + for (const forge of spellings) { + const root = await tmpDir("craftar-gap2-ctl-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + const forgeDir = path.join(root, "forge"); + const ws = path.join(root, "ws"); + await makeForge(forgeDir, { + ingredients: [rule("a", "# A\n")], + recipes: [recipe("base", ["rule/a"])], + profiles: [profile("acme", ["base"])], + }); + await fs.mkdir(ws); + await fs.writeFile(path.join(ws, "craftar.yaml"), `forge: ${JSON.stringify(forge)}\nprofile: acme\n`); + const home = path.join(root, "home"); + await fs.mkdir(home); + const r = runCli(["sync", "--workspace", ws], { env: { CRAFTAR_HOME: home } }); + const label = JSON.stringify(forge).replace(PW, "").replace(USER, ""); + expect(r.code, label).toBe(1); + expect(r.stderr, label).toContain("craftar.yaml › forge holds credentials in the URL"); + expect(r.stdout + r.stderr, label).not.toContain(PW); + expect(r.stdout + r.stderr, label).not.toContain(PW.toLowerCase()); + } + }); +}); + +describe("spec 39 gap 2 — the interactive init refuses the answer and does not offer it back", () => { + // One-slash spelling for the Forge answer + const ONE_SLASH = `https:/${CRED}example.invalid/acme/forge.git`; + + it("a one-slash spelling typed as the Forge is refused, and no question or line holds any part of it", async () => { + const { askInit } = await import("../src/core/init-flow.js"); + const root = await tmpDir("craftar-gap2-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + const home = path.join(root, "home"); + await fs.mkdir(home); + const asked: string[] = []; + const said: string[] = []; + const answers = [ONE_SLASH]; + const next = async (q: string) => (asked.push(q), answers.shift() ?? null); + const io = { ask: next, confirm: next, say: (l: string) => void said.push(l) }; + const result = await askInit(path.join(root, "ws"), { addRecipes: [], removeRecipes: [], replace: false }, { doc: null, keys: [] }, io, { home, registryOff: false }); + expect(result.kind).toBe("cancelled"); + expect(said).toContain("the answer holds credentials in the URL — remove them and let git authenticate"); + const all = JSON.stringify([asked, said]); + expect(all.includes(PW), "the password is printed or offered").toBe(false); + expect(all.includes(USER), "the user is printed or offered").toBe(false); + }); +}); diff --git a/test/doctor.test.ts b/test/doctor.test.ts index f30dfc2a..11564481 100644 --- a/test/doctor.test.ts +++ b/test/doctor.test.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { runDoctor, type Check, type DoctorOptions } from "../src/core/doctor.js"; import { register } from "../src/core/registry.js"; -import { cacheKey, ensureTree } from "../src/core/remote.js"; +import { cacheKey, ensureTree, UNVOUCHED_HIDDEN } from "../src/core/remote.js"; import { apply, loadWorkspace, plan, readLock, status } from "../src/core/sync.js"; import { makeForge, makeWorkspace, profile, recipe, rule, tmpDir, writeFiles, type ForgeSpec } from "./helpers/forge.js"; import { git, remoteForge } from "./helpers/remote.js"; @@ -136,7 +136,7 @@ describe("the cache check (§4.3)", () => { await fs.mkdir(path.join(c.home, "forges", "example.com-half-000000000000"), { recursive: true }); const r = await run({ home: c.home }); const found = of(r.checks, "cache"); - expect(found.map((x) => x.message).sort()).toEqual([`${c.key} — nothing names it`, "example.com-half-000000000000 — a fetch never completed"].sort()); + expect(found.map((x) => x.message).sort()).toEqual([`${UNVOUCHED_HIDDEN} — nothing names it`, `${UNVOUCHED_HIDDEN} — a fetch never completed`].sort()); expect(found.every((x) => x.level === "warn" && x.fix === "craftar cache prune")).toBe(true); }); @@ -156,7 +156,7 @@ describe("the cache check (§4.3)", () => { await fs.mkdir(path.join(c.home, "forges", "example.com-half-000000000000"), { recursive: true }); expect(one((await run({ home: c.home, registryOff: true })).checks, "cache")).toMatchObject({ level: "warn", - message: "example.com-half-000000000000 — a fetch never completed (orphans not checked: CRAFTAR_NO_REGISTRY is set)", + message: `${UNVOUCHED_HIDDEN} — a fetch never completed (orphans not checked: CRAFTAR_NO_REGISTRY is set)`, }); await fs.rm(path.join(c.home, "forges", "example.com-half-000000000000"), { recursive: true }); const bad = path.join(c.root, "bad"); @@ -169,7 +169,7 @@ describe("the cache check (§4.3)", () => { const a = await c.ws("acme-a", { profile: "acme", forge: "../nowhere" }); const r = await run({ home: c.home, workspace: a }); expect(one(r.checks, "config").level).toBe("error"); - expect(one(r.checks, "cache")).toMatchObject({ level: "warn", message: `${c.key} — nothing names it` }); + expect(one(r.checks, "cache")).toMatchObject({ level: "warn", message: `${UNVOUCHED_HIDDEN} — nothing names it` }); }); it("the size is summed", async () => { @@ -430,7 +430,7 @@ describe("the cache check and removal directories (spec 26 §5.1, §6 case 13)", const f = await fixture(); await writeFiles(path.join(f.home, "forges"), { "example.com-half-000000000000/repo.git/HEAD": "x" }); const r = await run({ home: f.home }); - expect(one(r.checks, "cache")).toEqual({ id: "cache", scope: "machine", level: "warn", message: "example.com-half-000000000000 — a fetch never completed", fix: "craftar cache prune" }); + expect(one(r.checks, "cache")).toEqual({ id: "cache", scope: "machine", level: "warn", message: `${UNVOUCHED_HIDDEN} — a fetch never completed`, fix: "craftar cache prune" }); }); }); diff --git a/test/hide-cache.test.ts b/test/hide-cache.test.ts new file mode 100644 index 00000000..24d36bf7 --- /dev/null +++ b/test/hide-cache.test.ts @@ -0,0 +1,126 @@ +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { hideCacheKeys, hideCacheSegment, hideKeys, hideUnvouched, vouchedKeys } from "../src/core/registry.js"; +import { CREDENTIAL_HIDDEN, UNVOUCHED_HIDDEN } from "../src/core/remote.js"; +import { RegistryEntrySchema } from "../src/schema/index.js"; + +const KEY = "fake" + "user-" + "nota" + "secret-example.invalid-f-0123456789ab"; + +describe("hideCacheSegment (spec 39 §5.1, the no-snapshot rule)", () => { + const cases: Array<[string, string]> = [ + ["a plain home", "/tmp/h"], + ["a home holding $&", "/tmp/h$&x"], + ["a home holding $1", "/tmp/h$1x"], + ["a home holding $` and $'", "/tmp/h$`$'x"], + ["a home holding regex characters", "/tmp/h.(+)*[x]"], + ["the root as home", "/"], + ]; + for (const [name, home] of cases) + it(`${name}: the segment after forges/ is the label, the key never printed`, () => { + const resolved = path.resolve(home); + const msg = `EACCES: permission denied, open '${path.join(resolved, "forges", KEY, "fetched")}'`; + const out = hideCacheSegment(msg, home); + expect(out.includes(KEY), out).toBe(false); + expect(out).toContain(UNVOUCHED_HIDDEN); + }); + it("either separator after forges, and a ~removing- name, are replaced whole", () => { + // The home as the file system reports it — path.resolve, so `D:\tmp\h` on win32 — with backslashes after it. + const out = hideCacheSegment(`rmdir '${path.resolve("/tmp/h")}\\forges\\~removing-${KEY}-4242\\repo.git'`, "/tmp/h"); + expect(out.includes(KEY)).toBe(false); + expect(out).toContain(UNVOUCHED_HIDDEN); + }); + it("text that names no cache entry is given back as it is", () => { + expect(hideCacheSegment("cannot read /tmp/h/forges: not a directory", "/tmp/h")).toBe("cannot read /tmp/h/forges: not a directory"); + }); +}); + +/** Build a minimal RegistryEntry, validated through the schema. */ +function entry(over: { + path?: string; + profile?: string; + forge: { kind: "path"; source: string; key: string | null } | { kind: "remote"; source: string; key: string; ref: string | null }; + lastSync?: string; +}) { + const base = { + path: over.path ?? "/ws", + profile: over.profile ?? "acme", + forge: { + kind: over.forge.kind, + source: over.forge.source, + key: over.forge.key, + ref: over.forge.kind === "remote" ? over.forge.ref : null, + commit: null, + fromLocalFile: false, + }, + recipes: ["base"], + stack: {}, + targets: ["claude-code"], + lastSync: over.lastSync ?? "2026-01-01T00:00:00.000Z", + }; + return RegistryEntrySchema.parse(base); +} + +describe("hideUnvouched and vouchedKeys (spec 39 §3, §5.1)", () => { + it("hideUnvouched replaces each key by the label and ignores an empty key", () => { + expect(hideUnvouched(`forges/${KEY}/trees`, [KEY])).toBe(`forges/${UNVOUCHED_HIDDEN}/trees`); + expect(hideUnvouched("forges/abc", [""])).toBe("forges/abc"); + }); + it("vouchedKeys: empty with no registry or no named map; a key both named and flagged is not vouched", () => { + const named = new Map([["clean-key", ["/ws/a"]], ["flagged-key", ["/ws/b"]]]); + expect(vouchedKeys(named, null).size).toBe(0); + expect(vouchedKeys(new Map(), { schema: 1, workspaces: [] } as never).size).toBe(0); + const reg = { schema: 1 as const, workspaces: [ + entry({ path: "/ws/a", forge: { kind: "remote", source: "https://example.invalid/a.git", key: "clean-key", ref: null } }), + entry({ path: "/ws/b", forge: { kind: "remote", source: "https://" + "u" + ":" + "p" + "@example.invalid/b.git", key: "flagged-key", ref: null } }), + ] }; + expect([...vouchedKeys(named, reg)]).toEqual(["clean-key"]); + }); +}); + +describe("one pass over every key (spec 39, review round 2)", () => { + const SHORT = "github.com-u"; + const LONG = "github.com-u-tok-" + "abcdef012345"; + it("a key that is a prefix of another never leaves the rest of the longer one printed", () => { + const text = `forges/${SHORT} and forges/${LONG}/trees`; + const out = hideUnvouched(text, [SHORT, LONG]); + expect(out).toBe(`forges/${UNVOUCHED_HIDDEN} and forges/${UNVOUCHED_HIDDEN}/trees`); + expect(out.includes("tok-")).toBe(false); + expect(hideKeys(text, [SHORT, LONG])).toBe(`forges/${CREDENTIAL_HIDDEN} and forges/${CREDENTIAL_HIDDEN}/trees`); + }); + it("a key that is a substring of a label never rewrites the inserted label", () => { + expect(hideUnvouched("forges/k1 forges/n", ["k1", "n"])).toBe(`forges/${UNVOUCHED_HIDDEN} forges/${UNVOUCHED_HIDDEN}`); + }); + it("hideCacheKeys: a credential key that is a prefix of an unvouched one — each gets its own label, nothing of either leaks", () => { + const out = hideCacheKeys(`forges/${SHORT} forges/${LONG}`, [SHORT], [LONG]); + expect(out).toBe(`forges/${CREDENTIAL_HIDDEN} forges/${UNVOUCHED_HIDDEN}`); + }); + it("empty keys are ignored by every form", () => { + expect(hideCacheKeys("forges/abc", [""], [""])).toBe("forges/abc"); + }); +}); + +describe("overlapping keys are hidden as one range (spec 39, review round 3)", () => { + const CRED = "github.com-u-tok-" + "abcdef012345"; + const DECOY = "removing-github"; + it("a key that starts before a credential key and ends inside it leaves nothing of the credential key printed", () => { + const text = `forges/${DECOY} forges/~removing-${CRED}-4242`; + for (const [cred, unv] of [[[CRED], [DECOY]], [[CRED, DECOY], []], [[], [CRED, DECOY]]] as const) { + const out = hideCacheKeys(text, cred, unv); + expect(out.includes("tok-")).toBe(false); + expect(out.includes("abcdef012345")).toBe(false); + expect(out.includes(".com-u")).toBe(false); + expect(out.endsWith("-4242")).toBe(true); + } + }); + it("one label per merged range — the credential label when a credential key covers any part of it", () => { + const text = `x ~removing-${CRED}-4242 y`; + expect(hideCacheKeys(text, [CRED], [DECOY])).toBe(`x ~${CREDENTIAL_HIDDEN}-4242 y`); + expect(hideCacheKeys(text, [], [CRED, DECOY])).toBe(`x ~${UNVOUCHED_HIDDEN}-4242 y`); + }); + it("adjacent but not overlapping keys keep a label each", () => { + expect(hideCacheKeys("abcdef", ["abc"], ["def"])).toBe(`${CREDENTIAL_HIDDEN}${UNVOUCHED_HIDDEN}`); + }); + it("a key inside an inserted label is never matched", () => { + expect(hideCacheKeys("forges/k1", ["k1"], ["credentials", "cache"])).toBe(`forges/${CREDENTIAL_HIDDEN}`); + }); +}); diff --git a/test/init-flow.test.ts b/test/init-flow.test.ts index 1fa08ad9..9cf34dd4 100644 --- a/test/init-flow.test.ts +++ b/test/init-flow.test.ts @@ -149,6 +149,13 @@ describe("lastForge", () => { const result = lastForge(reg, () => true); expect(result).toEqual({ forge: "/abs/forge", ref: null }); }); + it("spec 39 §14 item 8: a path row whose source is a one-slash credential spelling is not offered; a clean path row still is", () => { + const source = "https:/" + "fake" + "user" + ":" + "NOTA" + "SECRET" + "@example.invalid/acme/forge.git"; + const flagged = entry({ forge: { kind: "path", source, key: "/f/held" }, lastSync: "2026-01-03T00:00:00.000Z" }); + const clean = entry({ forge: { kind: "path", source: "../forge", key: "/f/b" }, lastSync: "2026-01-02T00:00:00.000Z" }); + expect(lastForge({ schema: 1, workspaces: [flagged, clean] }, () => true)).toEqual({ forge: "/f/b", ref: null }); + expect(lastForge({ schema: 1, workspaces: [flagged] }, () => true)).toBeNull(); + }); }); /* ------------------------------------------------------------------ */ diff --git a/test/printed-recorded-values.test.ts b/test/printed-recorded-values.test.ts index d9f932ec..55eec5aa 100644 --- a/test/printed-recorded-values.test.ts +++ b/test/printed-recorded-values.test.ts @@ -5,10 +5,11 @@ import { pruneCache } from "../src/core/cache.js"; import { runDoctor, type DoctorOptions } from "../src/core/doctor.js"; import { askInit, type InitIo } from "../src/core/init-flow.js"; import { credentialKeys, hideKeys, listWorkspaces, readRegistry, register, registryFile } from "../src/core/registry.js"; -import { cacheKey, credentialFault, ensureTree, parseLsRemote, resolveRef, type GitRunner } from "../src/core/remote.js"; +import { cacheKey, credentialFault, CREDENTIAL_HIDDEN, ensureTree, parseLsRemote, resolveRef, UNVOUCHED_HIDDEN, type GitRunner } from "../src/core/remote.js"; import { apply, loadForgeSource, loadWorkspace, loadWorkspaceConfig, plan, readLock, status } from "../src/core/sync.js"; import { makeForge, makeWorkspace, profile, recipe, rule, tmpDir, writeFiles } from "./helpers/forge.js"; import { remoteForge } from "./helpers/remote.js"; +import { runCli } from "./helpers/cli.js"; // 0.28.1 — three values that reached an output: a Forge URL with credentials that an earlier // release recorded in the registry, a `forge:` spelled with backslashes after the scheme, and a @@ -154,7 +155,24 @@ describe("a Forge URL with credentials recorded by an earlier release", () => { } }); - it("cache prune: an entry nothing with credentials names keeps its name", async () => { + it("cache prune: an entry a clean registry row names keeps its name (spec 39, pinned)", async () => { + const root = await tmpDir("craftar-recorded-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + const home = path.join(root, "home"); + await fs.mkdir(home); + const r = await remoteForge(SPEC); + cleanups.push(r.cleanup); + const dir = path.join(root, "acme-clean"); + await makeWorkspace(dir, root, { config: { profile: "acme", forge: r.url } }); + const w = await loadWorkspace(dir, { home }); + const p = await plan(w); + await apply(w, p, await status(w, p, await readLock(w.root))); + await register(home, w, p); + const { report } = await pruneCache(home, { dryRun: true, workspace: null, registryOff: false }); + expect(report.kept.map((x) => x.path)).toEqual([`forges/${cacheKey(r.url)}`]); + }); + + it("cache prune: an entry nothing names is named by the unvouched label (spec 39 §4.1 a)", async () => { const root = await tmpDir("craftar-recorded-"); cleanups.push(() => fs.rm(root, { recursive: true, force: true })); const home = path.join(root, "home"); @@ -163,7 +181,7 @@ describe("a Forge URL with credentials recorded by an earlier release", () => { cleanups.push(r.cleanup); await ensureTree(r.url, null, { home }); const { report } = await pruneCache(home, { dryRun: true, workspace: null, registryOff: false }); - expect(report.removed.map((x) => x.path)).toEqual([`forges/${cacheKey(r.url)}`]); + expect(report.removed.map((x) => x.path)).toEqual([`forges/${UNVOUCHED_HIDDEN}`]); }); it("doctor: the cache check names such an entry by the label", async () => { @@ -377,3 +395,125 @@ describe("a ref: that holds a URL with credentials", () => { } }); }); + +describe("spec 39 gap 1: a cache entry named after a URL with credentials, that no registry row names any more", () => { + const UNVOUCHED = "(a cache entry name — not shown)"; + + /** recordedHome, then the registry rows dropped: the state right after the user re-synced without the credentials and pruned the registry. */ + async function orphanHome() { + const h = await recordedHome(); + const file = registryFile(h.home); + const reg = JSON.parse(await fs.readFile(file, "utf8")); + reg.workspaces = []; + await fs.writeFile(file, JSON.stringify(reg, null, 2) + "\n"); + return h; + } + const doctorOpts = (home: string, extra: Partial = {}): DoctorOptions => ({ + version: "0.0.0", workspace: null, fetch: false, strict: false, registryOff: false, env: {}, nodeVersion: "22.11.0", + gitVersion: async () => "2.53.0", home, ...extra, + }); + + it("the label's text is pinned", () => expect(UNVOUCHED_HIDDEN).toBe(UNVOUCHED)); + + it("doctor: `nothing names it` and `a fetch never completed` name it by the label — text and the report object", async () => { + const h = await orphanHome(); + const report = await runDoctor(doctorOpts(h.home)); + expect(report.checks.filter((c) => c.id === "cache").map((c) => c.message)).toEqual([`${UNVOUCHED} — nothing names it`]); + expectClean(report); + await fs.rm(path.join(h.home, "forges", h.oldKey, "fetched")); + const again = await runDoctor(doctorOpts(h.home)); + expect(again.checks.filter((c) => c.id === "cache").map((c) => c.message)).toEqual([`${UNVOUCHED} — a fetch never completed`]); + expectClean(again); + }); + + it("cache prune: the entry, its ~removing- leftover and a warning about it are named by the label — dry run and run", async () => { + for (const dryRun of [true, false]) { + const h = await orphanHome(); + await fs.mkdir(path.join(h.home, "forges", `~removing-${h.oldKey}-4242`)); + const { report } = await pruneCache(h.home, { dryRun, workspace: null, registryOff: false }); + expect(report.removed.map((r) => r.path).sort()).toEqual([`forges/${UNVOUCHED}`, `forges/~removing-${UNVOUCHED}-4242`].sort()); + expectClean(report); + } + const h = await orphanHome(); + await fs.rm(path.join(h.home, "forges", h.oldKey, "fetched")); + const rename = async () => { + throw Object.assign(new Error("held"), { code: "EPERM" }); + }; + const { report } = await pruneCache(h.home, { dryRun: false, workspace: null, registryOff: false, rename }); + expect(report.warnings).toEqual([`${UNVOUCHED} is held open (EPERM) — kept`]); + expectClean(report); + }); + + it("with CRAFTAR_NO_REGISTRY every entry is labelled — even one a row with credentials names", async () => { + const h = await recordedHome(); // its rows still name the entry, flagged + const { report } = await pruneCache(h.home, { dryRun: true, workspace: null, registryOff: true }); + expect([...report.removed, ...report.kept].map((r) => r.path)).toEqual([`forges/${UNVOUCHED}`]); + expectClean(report); + // Remove the fetched stamp to make the entry incomplete, so doctor reports it individually. + await fs.rm(path.join(h.home, "forges", h.oldKey, "fetched")); + const d = await runDoctor(doctorOpts(h.home, { registryOff: true })); + // The cache check reports the entry by the label because no row vouches it (registry off). + expect(d.checks.filter((c) => c.id === "cache").map((c) => c.message)).toEqual([ + `${UNVOUCHED} — a fetch never completed (orphans not checked: CRAFTAR_NO_REGISTRY is set)`, + ]); + expectClean(d); + }); + + it("no snapshot: doctor's unreadable-cache line and cache prune's failed snapshot print the label, a flagged key included", async () => { + const h = await recordedHome(); // flagged key, named by its rows + const inside = path.join(h.home, "forges", h.oldKey, "fetched"); + const failing = async () => { + throw Object.assign(new Error(`EACCES: permission denied, open '${inside}'`), { code: "EACCES" }); + }; + const d = await runDoctor(doctorOpts(h.home, { inspectCache: failing } as Partial)); + const line = d.checks.find((c) => c.id === "cache")!.message; + expect(line).toBe(`cannot read the Forge cache: EACCES: permission denied, open '${path.join(path.resolve(h.home), "forges", UNVOUCHED, "fetched")}'`); + expectClean(d); + const message = await pruneCache(h.home, { dryRun: true, workspace: null, registryOff: false, inspectCache: failing } as Parameters[1]).then( + () => "", + (e: unknown) => (e as Error).message, + ); + expect(message).toContain(UNVOUCHED); + expect(message).not.toContain(LABEL); + expectClean(message); + }); + + it("spec 39 §7: cache prune CLI text output with a labelled tree row does not throw in labelOf", async () => { + // Build a home with a cache entry whose name holds a credential (flagged rows name it). + const h = await recordedHome(); + + // Find the tree inside the renamed cache entry — sync created it. + const treesDir = path.join(h.home, "forges", h.oldKey, "trees"); + const treeEntries = await fs.readdir(treesDir); + const commit = treeEntries.find((e) => !e.includes(".")); + expect(commit, "no tree commit found").toBeDefined(); + + // Make the tree's .used stamp 15 days old so cache prune --dry-run lists it. + const DAY = 24 * 60 * 60 * 1000; + const ago15 = new Date(Date.now() - 15 * DAY); + await fs.utimes(path.join(treesDir, `${commit}.used`), ago15, ago15); + + // Run the CLI from a directory without craftar.yaml (as other cache-prune CLI tests do). + const r = runCli(["cache", "prune", "--dry-run"], { env: { CRAFTAR_HOME: h.home }, cwd: h.root }); + expect(r.code, r.stderr).toBe(0); + + // The tree row uses the label, not the raw key. + expect(r.stdout).toContain(LABEL); + expect(r.stdout).toContain("/trees/"); + expectClean(r.stdout); + expectClean(r.stderr); + }); + + it("spec 39 §7: cache prune CLI text output with CRAFTAR_NO_REGISTRY shows the unvouched label", async () => { + const h = await recordedHome(); + // Run the CLI with the registry off, so every entry is unvouched. + const r = runCli(["cache", "prune", "--dry-run"], { env: { CRAFTAR_HOME: h.home, CRAFTAR_NO_REGISTRY: "1" }, cwd: h.root }); + expect(r.code, r.stderr).toBe(0); + // The entry row uses the unvouched label (neither label contains "/"). + expect(r.stdout).toContain(UNVOUCHED); + expect(UNVOUCHED).not.toContain("/"); + expect(LABEL).not.toContain("/"); + expectClean(r.stdout); + expectClean(r.stderr); + }); +}); diff --git a/test/remote.test.ts b/test/remote.test.ts index 2ea90286..c83e8cc8 100644 --- a/test/remote.test.ts +++ b/test/remote.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { cacheKey, classifyForge, credentialFault, parseLsRemote, resolveRef } from "../src/core/remote.js"; +import { cacheKey, classifyForge, credentialFault, parseLsRemote, refHoldsCredentials, resolveRef } from "../src/core/remote.js"; describe("what counts as a URL (spec 13 §6.1)", () => { it("URLs", () => { @@ -95,6 +95,33 @@ describe("credentials (spec 13 §4.3, §6.1)", () => { expect(credentialFault(v)).toBe(true); expect(cacheKey(v)).toContain("pw" + "x".repeat(6)); }); + // Spec 39 §4.2 (A) and (B): a scheme followed by one slash, or a tab / line break inside the scheme or its `://`, used to be + // read as a path and printed whole. Fake parts, assembled at runtime. + const U = "fake" + "user"; + const P = "NOTA" + "SECRET"; + const H = "example.invalid/acme/forge.git"; + it("refused (spec 39): one slash, and tab / line feed / carriage return inside the scheme or its ://", () => { + for (const v of [ + `https:/${U}:${P}@${H}`, + `https:/${U}@${H}`, + `http:/${U}:${P}@${H}`, + `ssh:/${U}:${P}@${H}`, + `https:\t//${U}:${P}@${H}`, + `https:/\n/${U}@${H}`, + `ht\ttps://${U}@${H}`, + `https:\r//${U}:${P}@${H}`, + `https:/\r/${U}@${H}`, + `ht\rtps://${U}@${H}`, + `C:/x:y@z`, + ]) + expect(credentialFault(v), JSON.stringify(v.replace(P, ""))).toBe(true); + // refHoldsCredentials is used for the README's "a ref: spelled that way is named by the label". + expect(refHoldsCredentials(`https:/${U}:${P}@${H}`)).toBe(true); + }); + it("accepted (spec 39, pinned): paths, a bare user on a non-http scheme, file, a tab in a path", () => { + for (const v of ["C:/forges/acme", "../acme-forge", "foo:/a@b", "file:/x@y", "ssh:/git@example.invalid/f", "../my\tforge"]) + expect(credentialFault(v), JSON.stringify(v)).toBe(false); + }); }); describe("the cache key (spec 13 §6.3)", () => { diff --git a/test/ui-routes.test.ts b/test/ui-routes.test.ts index 4c39fe82..0681f86b 100644 --- a/test/ui-routes.test.ts +++ b/test/ui-routes.test.ts @@ -491,6 +491,19 @@ describe("a Forge URL recorded with credentials is never served (spec 36 §7.5)" expect(servedRow(loaded)).toEqual({ id: workspaceId(loaded.path), ...loaded }); }); + + it("spec 39: a recorded forge.source spelled with one slash after the scheme is served as the label, with no key", async () => { + const s = kept(await scenario(BASE, { config: { profile: "acme" } })); + await syncRegistered(s.wsRoot); + const loaded = (await listWorkspaces(HOME(), { fetch: false })).rows[0]; + const oneSlash = "https:/" + USER + ":" + PW + "@forge.example.invalid/acme"; + const served = servedRow({ ...loaded, forge: { ...loaded.forge, kind: "remote" as const, source: oneSlash, key: "forge.example.invalid-x" } }); + expect(served.forge.source).toBe(CREDENTIAL_HIDDEN); + expect(served.forge.key).toBeNull(); + const json = JSON.stringify(served); + expect(json).not.toContain(USER); + expect(json).not.toContain(PW); + }); it("a recorded ref that holds a URL with credentials is served as the label, the rest of the row untouched", async () => { const s = kept(await scenario(BASE, { config: { profile: "acme" } })); await syncRegistered(s.wsRoot); diff --git a/test/url-credential.test.ts b/test/url-credential.test.ts index 6921a20f..422889c3 100644 --- a/test/url-credential.test.ts +++ b/test/url-credential.test.ts @@ -59,6 +59,12 @@ const FINDING: Array<[string, string]> = [ ["the empty authority and a line feed before the @, together", "https:///tok\n@host/"], ["refused though it is no secret: a URL, a tab, a handle", "https://example.com\t@handle"], ["a tab inside the scheme does not save a password", "ht\ttps://u:p@host/"], + // Spec 39 §4.2 (B′): a tab or a line break inside the scheme or its :// — a URL parser removes them, so the userinfo reaches the host. + ["a tab inside the ://", "https:\t//u:p@host/"], + ["a tab inside the scheme, a bare user", "ht\ttps://tok@host/"], + ["a line feed inside the scheme, a bare user", "ht\ntps://tok@host/"], + ["a line feed between the two slashes", "https:/\n/u:p@host/"], + ["a word, a line feed, then a URL with a bare user", "see\nhttps://tok@host/"], ]; const NONE: Array<[string, string]> = [ @@ -129,8 +135,6 @@ const NONE: Array<[string, string]> = [ ["a host and a port, a space, a mention", "postgres://db.internal:5432 @replica"], ["a line feed and no @ after it", "https://tok\n\n\n"], ["ssh, a line feed, a mention — no colon before the @", "ssh://host\n@alice"], - ["a tab inside the :// — still passes, though it holds a password", "https:\t//u:p@host/"], - ["a tab inside the scheme, a bare user — still passes", "ht\ttps://tok@host/"], ]; describe("urlCredentialIn — row U", () => { @@ -147,4 +151,8 @@ describe("urlCredentialIn — row U", () => { [false, true], ]); }); + it("the removal is confined to the scheme and its :// (spec 39 §5.2): the pinned passes stay passes", () => { + for (const text of ["https://tok\n @host/", "https://u\t:p@host/", "ssh://git\n@host/x", "https://example.com\nops@example.com", "site: https://example.com\nmail: ops@example.com", "ssh://host\n@alice"]) + expect(urlCredentialIn(text), JSON.stringify(text)).toBe(false); + }); });