Fix docref issues #1–#4 and open-review findings

.github/workflows/open-review.yml

@@ -39,9 +39,14 @@ jobs:
           OR_LLM_API_KEY: ${{ secrets.OR_LLM_API_KEY }}
           GH_TOKEN: ${{ github.token }}
         run: |
+          # The container's git runs as root over the runner-owned checkout, so
+          # git's dubious-ownership guard rejects it ("Not a git repository") and
+          # open-review crashes before it can diff. Mark /repo safe via GIT_CONFIG_*
+          # env (applies to every git call inside the container, no config file).
           docker run --rm --env-file or.env \
             -e OR_LLM_API_KEY -e GITHUB_TOKEN="$GH_TOKEN" \
             -e GITHUB_ACTIONS=true -e GITHUB_BASE_REF="${{ github.base_ref }}" \
+            -e GIT_CONFIG_COUNT=1 -e GIT_CONFIG_KEY_0=safe.directory -e GIT_CONFIG_VALUE_0=/repo \
             -v "$PWD":/repo -w /repo \
             ghcr.io/manchtools/open-review:latest run
           # gate, sarif, provider pin, --emit-* etc. all come from OR_* env — no flags here.

README.md

@@ -48,7 +48,7 @@ source with `docref: begin <name>` / `docref: end <name>`, for sub-symbol slices
 and languages without addressable symbols, such as CSS). The code side has its
 own loud signal, not just the docs:
 
-<!-- docref: begin src=packages/core/src/ops.ts#anchors:8d9d6ba5,packages/core/src/ops.ts#exitCode:d41a5364 -->
+<!-- docref: begin src=packages/core/src/ops.ts#anchors:99f82d16,packages/core/src/ops.ts#exitCode:d41a5364 -->
 `docref anchors` inventories every region marker and what references it, and a
 marker that nothing references is flagged **not used** — which fails `docref
 check` unless `[anchors] allow-unused = true`. A stranded marker, like a broken

docs/01-getting-started/01-introduction.md

@@ -48,7 +48,7 @@ no marker needed) or a **region** (a span named in the source with
 `docref: begin <name>` / `docref: end <name>`, for sub-symbol slices). The code
 side has its own loud signal, not just the docs:
 
-<!-- docref: begin src=packages/core/src/ops.ts#anchors:8d9d6ba5,packages/core/src/ops.ts#exitCode:d41a5364 -->
+<!-- docref: begin src=packages/core/src/ops.ts#anchors:99f82d16,packages/core/src/ops.ts#exitCode:d41a5364 -->
 `docref anchors` inventories every region marker and what references it, and a
 marker that nothing references is flagged **not used**, which fails
 `docref check` unless `[anchors] allow-unused = true`. A stranded marker, like a

docs/03-tooling/01-cli/03-cross-repo-update.md

@@ -7,7 +7,7 @@ description: Advance cross-repo pins in docref.lock to a tracked branch's tip, r
 
 ## `docref update [alias...] [--check]`
 
-<!-- docref: begin src=packages/core/src/ops.ts#update:8dc77057 -->
+<!-- docref: begin src=packages/core/src/ops.ts#update:aaadde47 -->
 
 For each alias (default: all): fetch the tracked branch, advance `docref.lock` to its tip, refresh all snippets referencing the alias, and report every claim that became `stale-claim` under the new rev.
 

docs/03-tooling/01-cli/05-ls-and-anchors.md

@@ -15,7 +15,7 @@ Dump the reverse index: every referenced anchor and everything referencing it. T
 
 ## `docref anchors [--json]`
 
-<!-- docref: begin src=packages/core/src/ops.ts#anchors:8d9d6ba5 -->
+<!-- docref: begin src=packages/core/src/ops.ts#anchors:99f82d16 -->
 
 The code-side inventory, the reverse of `ls`: scan the source tree for every declared region marker and list each with the references to it; an anchor with none is flagged **not used**. Marker errors (duplicate names, unmatched begin/end) surface here even in files nothing references, which `check` alone would never visit. Exit `2` on marker errors, `0` otherwise (an unused anchor is information, not a failure).
 

docs/04-internals/04-parsing-a-reference.md

@@ -61,14 +61,8 @@ export function parseRef(raw: string): Ref {
 Run from anywhere inside the repository: `docref` walks up to the nearest
 `docref.toml`, falling back to the working directory.
 
-```ts docref=packages/core/src/config.ts#findRoot:371ad98d
+```ts docref=packages/core/src/config.ts#findRoot:64be4c67
 export function findRoot(cwd: string): string {
-	let dir = cwd;
-	for (;;) {
-		if (existsSync(join(dir, 'docref.toml'))) return dir;
-		const parent = dirname(dir);
-		if (parent === dir) return cwd;
-		dir = parent;
-	}
+	return findRootOrNull(cwd) ?? cwd;
 }
 ```

install.sh

@@ -56,10 +56,10 @@ download_asset() {
 		# select the asset object by name and read its API url as a unit — never
 		# rely on name/url field adjacency (the url precedes the name in the JSON)
 		url=$(printf '%s' "$release_json" | jq -r --arg n "$name" '.assets[] | select(.name==$n) | .url')
-		[ -n "$url" ] && [ "$url" != "null" ] || {
+		if [ -z "$url" ] || [ "$url" = "null" ]; then
 			echo "docref: no asset '$name' in the latest release." >&2
 			exit 1
-		}
+		fi
 		curl -fSL -H "Authorization: Bearer $GITHUB_TOKEN" -H "Accept: application/octet-stream" "$url" -o "$out"
 	else
 		curl -fSL "https://github.com/${repo}/releases/latest/download/${name}" -o "$out"
@@ -106,7 +106,7 @@ trap - EXIT
 rm -f "$sums"
 
 echo "docref: installed to $dir/docref"
-"$dir/docref" --version >/dev/null 2>&1 && echo "docref: $($dir/docref --version) ready — run 'docref check', update with 'docref self-update'."
+"$dir/docref" --version >/dev/null 2>&1 && echo "docref: $("$dir/docref" --version) ready — run 'docref check', update with 'docref self-update'."
 
 case ":$PATH:" in
 	*":$dir:"*) ;;

packages/cli/src/selfupdate.test.ts

@@ -30,7 +30,13 @@ describe('assetName', () => {
 // Covers correct / absent / present-but-wrong for each step, including the
 // checksum gate that guards the binary swap.
 describe('selfUpdate: decision and failure paths', () => {
-	const presentAsset = assetName(process.platform, process.arch);
+	// A fixed supported platform, injected into the IO, keeps these tests
+	// deterministic and independent of the running platform — so collection never
+	// throws on an unsupported host (assetName evaluated at describe scope would
+	// crash the whole file on, e.g., FreeBSD). The real platform matrix is
+	// covered in the assetName describe above.
+	const [PLATFORM, ARCH] = ['linux', 'x64'];
+	const presentAsset = assetName(PLATFORM, ARCH);
 	const BINARY = Buffer.from('new-binary');
 	const BIN_SHA = createHash('sha256').update(BINARY).digest('hex');
 	// a valid SHA256SUMS manifest matching BINARY for this platform's asset
@@ -46,8 +52,8 @@ describe('selfUpdate: decision and failure paths', () => {
 		downloadAsset: async (asset) => (asset.name === 'SHA256SUMS' ? SUMS : BINARY),
 		replaceBinary: () => {},
 		refreshExtension: async () => ({ code: 0, out: 'extension refreshed' }),
-		platform: process.platform,
-		arch: process.arch,
+		platform: PLATFORM,
+		arch: ARCH,
 		execPath: '/fake/docref',
 		...over
 	});

packages/cli/src/standalone.ts

@@ -24,7 +24,10 @@ import phpWasm from 'tree-sitter-wasms/out/tree-sitter-php.wasm' with { type: 'f
 import swiftWasm from 'tree-sitter-wasms/out/tree-sitter-swift.wasm' with { type: 'file' };
 import kotlinWasm from 'tree-sitter-wasms/out/tree-sitter-kotlin.wasm' with { type: 'file' };
 import scalaWasm from 'tree-sitter-wasms/out/tree-sitter-scala.wasm' with { type: 'file' };
-import bashWasm from 'tree-sitter-wasms/out/tree-sitter-bash.wasm' with { type: 'file' };
+// bash and proto are vendored (source-built): tree-sitter-wasms ships no proto,
+// and its bash build imports a libc symbol the runtime lacks, crashing the
+// external scanner (issue #1) — the vendored copy resolves it.
+import bashWasm from '@open-docref/core/grammars/tree-sitter-bash.wasm' with { type: 'file' };
 import protoWasm from '@open-docref/core/grammars/tree-sitter-proto.wasm' with { type: 'file' };
 import { configureWasm } from '@open-docref/core';
 import { run, VERSION } from './main';

packages/core/src/config.test.ts

@@ -1,7 +1,7 @@
 import { describe, it, expect } from 'vitest';
 import { mkdirSync } from 'node:fs';
 import { join } from 'node:path';
-import { loadProject, findRoot } from './config';
+import { loadProject, findRoot, findRootOrNull } from './config';
 import { DocrefError } from './errors';
 import { tmp, write } from './testutil';
 
@@ -123,4 +123,19 @@ describe('findRoot: locating the project root', () => {
 		// (parent === dir) and returns the original cwd, never looping forever
 		expect(findRoot(nested)).toBe(nested);
 	});
+
+	it('findRootOrNull returns the governing root, or null when there is none', () => {
+		// the governing-root rule the extension needs: a file belongs to the
+		// nearest docref.toml above it, independent of any workspace folder. In a
+		// multi-repo layout each sibling resolves to its OWN root, not a shared one.
+		const ws = tmp(); // the "workspace folder" — no docref.toml of its own
+		write(ws, 'sdk/docref.toml', '');
+		write(ws, 'docs/docref.toml', '');
+		mkdirSync(join(ws, 'sdk', 'pkg'), { recursive: true });
+		mkdirSync(join(ws, 'docs', 'content'), { recursive: true });
+		expect(findRootOrNull(join(ws, 'sdk', 'pkg'))).toBe(join(ws, 'sdk'));
+		expect(findRootOrNull(join(ws, 'docs', 'content'))).toBe(join(ws, 'docs'));
+		// the workspace folder itself governs nothing (no docref.toml at or above)
+		expect(findRootOrNull(ws)).toBeNull();
+	});
 });

packages/core/src/config.ts

@@ -125,13 +125,23 @@ export function appendRepoBlock(root: string, alias: string, url: string, ref?:
 	writeFileSync(path, prefix + block + '\n');
 }
 
-/** Walk up from cwd to the nearest docref.toml; fall back to cwd itself. */
-export function findRoot(cwd: string): string {
-	let dir = cwd;
+/**
+ * Walk up from `from` to the nearest directory containing docref.toml, or null
+ * if none is found before the filesystem root. This is the governing-root rule
+ * (ESLint/EditorConfig style): a file's project is the closest docref.toml at or
+ * above it, independent of any editor's workspace folder.
+ */
+export function findRootOrNull(from: string): string | null {
+	let dir = from;
 	for (;;) {
 		if (existsSync(join(dir, 'docref.toml'))) return dir;
 		const parent = dirname(dir);
-		if (parent === dir) return cwd;
+		if (parent === dir) return null;
 		dir = parent;
 	}
 }
+
+/** Walk up from cwd to the nearest docref.toml; fall back to cwd itself. */
+export function findRoot(cwd: string): string {
+	return findRootOrNull(cwd) ?? cwd;
+}

packages/core/src/gitcache.test.ts

@@ -105,6 +105,14 @@ describe('assertUrl: a remote url safe to hand git', () => {
 		expect(() => assertUrl('--upload-pack=touch /tmp/pwned')).toThrow(DocrefError);
 	});
 
+	it('rejects an scp-like url whose host or path is option-shaped (ssh -oProxyCommand RCE)', () => {
+		// host starting with '-' becomes an ssh option (CVE-2017-1000117 class)
+		expect(() => assertUrl('git@-oProxyCommand=touch /tmp/pwned:repo.git')).toThrow(DocrefError);
+		// path starting with '-' can reach the remote git as an option
+		expect(() => assertUrl('git@github.com:--upload-pack=touch /tmp/pwned')).toThrow(DocrefError);
+		expect(() => assertUrl('git@github.com:-x')).toThrow(DocrefError);
+	});
+
 	it('rejects empty and unknown/no scheme', () => {
 		expect(() => assertUrl('')).toThrow(DocrefError);
 		expect(() => assertUrl('javascript:alert(1)')).toThrow(DocrefError);

packages/core/src/gitcache.ts

@@ -43,7 +43,12 @@ export function cacheDirFor(url: string): string {
 const REV = /^[0-9a-f]{7,64}$/i;
 const REF = /^[0-9A-Za-z._/-]+$/;
 const URL_SCHEME = /^(https?|ssh|git|file):\/\//i;
-const SCP_LIKE = /^[^\s/:-][^\s/:]*@[^\s/:]+:.+$/;
+// scp-like `user@host:path`. The first char of user, host, AND path all exclude
+// '-': git hands the host to ssh, where a leading-dash host is parsed as an
+// option (`-oProxyCommand=<cmd>`, the CVE-2017-1000117 RCE class), and a
+// leading-dash path can reach the remote git as an option. The path also forbids
+// whitespace so a `host:--flag <cmd>` payload cannot smuggle a second token.
+const SCP_LIKE = /^[^\s/:-][^\s/:]*@[^\s/:-][^\s/:]*:[^\s-][^\s]*$/;
 
 export function assertRev(rev: string): string {
 	if (typeof rev !== 'string' || !REV.test(rev)) {

packages/core/src/index.ts

@@ -18,7 +18,7 @@ export {
 export { listDeclarations, findSymbol, configureWasm, type Decl, type WasmConfig } from './symbols';
 export { languageForFile, fenceLanguageForRef, type LanguageId, type LanguageInfo } from './languages';
 export { workingTreeSource, resolveAnchor, type FileSource, type Anchor } from './resolve';
-export { loadProject, writeLock, findRoot, GATE_LEVELS, type Project, type RepoConfig, type GateLevel } from './config';
+export { loadProject, writeLock, findRoot, findRootOrNull, GATE_LEVELS, type Project, type RepoConfig, type GateLevel } from './config';
 export { ensureCommit, branchTip, gitRevSource } from './gitcache';
 export {
 	check,

packages/core/src/ops.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
-import { existsSync, rmSync } from 'node:fs';
+import { existsSync, rmSync, chmodSync } from 'node:fs';
 import { join } from 'node:path';
 import { loadProject } from './config';
 import { check, refresh, approve, update, addRepo, affected, ls, anchors, diff, remove, exitCode, exitCodeFor, suggest, type Report } from './ops';
@@ -321,6 +321,56 @@ describe('affected: mapping changes to carriers', () => {
 	});
 });
 
+describe('path scoping: a directory argument (issue #2)', () => {
+	// A directory passed to a path-scoped op used to hit readFileSync and surface
+	// a raw EISDIR. It now expands to every scanned doc under it (the natural way
+	// to gate one section of a docs tree); a file is still taken verbatim.
+	// each doc carries one snippet ref to a missing file, so it surfaces as a
+	// broken entry keyed by its own doc path — the signal that proves which docs
+	// a given scope actually visited.
+	const brokenRef = (name: string) => ['```txt docref=' + name + '.txt#@x', '```', ''].join('\n');
+	function tree(): string {
+		const root = tmp();
+		write(root, 'content/01-intro/a.md', brokenRef('a'));
+		write(root, 'content/02-concepts/b.md', brokenRef('b'));
+		write(root, 'content/02-concepts/nested/c.md', brokenRef('c'));
+		write(root, 'other/d.md', brokenRef('d'));
+		return root;
+	}
+	const docsIn = (r: Report) => new Set(r.entries.map((e) => e.doc));
+
+	it('scans every doc under the directory instead of throwing EISDIR', async () => {
+		const root = tree();
+		const report = await check(loadProject(root), ['content/02-concepts']);
+		// the directory (including its nested doc) is in scope; siblings are not
+		expect(docsIn(report)).toEqual(
+			new Set(['content/02-concepts/b.md', 'content/02-concepts/nested/c.md'])
+		);
+	});
+
+	it('a trailing slash is tolerated and a nested directory scopes tighter', async () => {
+		const root = tree();
+		expect(docsIn(await check(loadProject(root), ['content/02-concepts/']))).toEqual(
+			new Set(['content/02-concepts/b.md', 'content/02-concepts/nested/c.md'])
+		);
+		expect(docsIn(await check(loadProject(root), ['content/02-concepts/nested']))).toEqual(
+			new Set(['content/02-concepts/nested/c.md'])
+		);
+	});
+
+	it('still checks a single explicit file verbatim', async () => {
+		const root = tree();
+		expect(docsIn(await check(loadProject(root), ['content/01-intro/a.md']))).toEqual(
+			new Set(['content/01-intro/a.md'])
+		);
+	});
+
+	it('keeps a nonexistent path so the read fails loudly (ENOENT), not silently dropped', async () => {
+		const root = tree();
+		await expect(check(loadProject(root), ['content/missing.md'])).rejects.toThrow();
+	});
+});
+
 describe('anchors: the code-side inventory', () => {
 	// The reverse view (tooling.md): every declared region marker in the
 	// codebase, each flagged with its references or as not used. Symbols
@@ -379,6 +429,24 @@ describe('anchors: the code-side inventory', () => {
 		const result = await anchors(loadProject(root));
 		expect(result.anchors.map((a) => a.name)).toEqual(['mine']);
 	});
+
+	it('lists markers inside dot-directories, so an unused one there is flagged', async () => {
+		// regression for issue #3: the code-side scan skipped dot-directories
+		// (.github/ especially) while the reference resolver read them by path, so
+		// a marker nothing references was never reported as unused — the fail-loud
+		// guarantee had a hole exactly where CI configs live.
+		const root = tmp();
+		write(
+			root,
+			'.github/workflows/release.yml',
+			'# docref: begin release-trigger\non: push\n# docref: end release-trigger\n'
+		);
+		const result = await anchors(loadProject(root));
+		expect(result.errors).toEqual([]);
+		const ref = '.github/workflows/release.yml#@release-trigger';
+		expect(result.anchors.map((a) => `${a.file}#@${a.name}`)).toContain(ref);
+		expect(result.anchors.find((a) => a.name === 'release-trigger')!.references).toEqual([]);
+	});
 });
 
 describe('unused anchors fail the gate', () => {
@@ -798,6 +866,32 @@ describe('cross-repo: lock, update, pinned resolution', () => {
 		expect(u2.report.summary.staleSnippet).toBe(0);
 	});
 
+	it('restores the in-memory lock when refresh fails mid-update', async () => {
+		// update advances project.lock in-memory before refresh and persists only
+		// after; if refresh throws, the on-disk lock stays put AND the in-memory
+		// lock is rolled back, so a reused Project is never left advanced past disk.
+		const { remote, root } = remoteFixture();
+		await update(loadProject(root)); // establish a prior locked rev
+		const p = loadProject(root);
+		const priorRev = p.lock.lib!.rev;
+		const lockOnDisk = read(root, 'docref.lock');
+
+		// the remote moves, so the next update wants to refresh the snippet
+		write(remote, 'src/handler.go', 'package api\n\nfunc Verify() { /* v2 */ }\n');
+		commitAll(remote, 'v2');
+		// make the doc unwritable so refresh's writeFileSync throws mid-update
+		chmodSync(join(root, 'docs/x.md'), 0o444);
+		try {
+			await expect(update(p)).rejects.toThrow();
+			// in-memory lock rolled back to the prior rev, not the new tip
+			expect(p.lock.lib!.rev).toBe(priorRev);
+			// and the on-disk lock was never advanced
+			expect(read(root, 'docref.lock')).toBe(lockOnDisk);
+		} finally {
+			chmodSync(join(root, 'docs/x.md'), 0o644);
+		}
+	});
+
 	it('fails closed at load on a poisoned lock rev instead of running git on it', () => {
 		const { remote, root } = remoteFixture();
 		// a malicious PR could ship a docref.lock whose rev is a git option;