fix(windows): use native path boundaries and assertions (#3578)

Co-Authored-By: Diego <diegodearagao@gmail.com>
This commit is contained in:
Garry Tan
2026-08-01 10:06:17 +08:00
committed by Sina Matian
co-authored by Diego
parent 522cfb032a
commit e77a15f0d8
25 changed files with 442 additions and 154 deletions
+1 -1
View File
@@ -153,7 +153,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this
- `src/commands/book-mirror.ts``gbrain book-mirror --chapters-dir <path> --slug <slug> [flags]`. Submits N read-only subagent jobs (one per chapter; `allowed_tools: ['get_page', 'search']`), waits for all via `waitForCompletion`, reads each child's `job.result`, assembles two-column markdown CLI-side, writes a single operator-trust `put_page` to `media/books/<slug>-personalized.md`. Trust narrowing happens at the tool-allowlist layer (subagents can't call put_page) so untrusted EPUB content can't prompt-inject any people page. Cost-estimate prompt before launching; refuses to spend in non-TTY without `--yes`. Per-chapter idempotency keys (`book-mirror:<slug>:ch-<N>`) for retry-friendly re-runs. Partial-failure: assembles completed chapters + a `## Failed chapters` section. Pinned by `test/book-mirror.test.ts` (9 cases).
- `src/commands/skillpack.ts` + `src/core/skillpack/{bundle,scaffold,reference,migrate-fence,scrub-legacy,harvest,harvest-lint,copy,apply-hunks,diff-text,installer}.ts` — managed-block install model retired; `install`/`uninstall` removed (exit non-zero with a hint to the replacement). Surface: `scaffold` (one-time additive copy via `copyArtifacts` in `copy.ts`; refuses to overwrite; partial-state fills missing paired sources declared in SKILL.md frontmatter `sources:`), `reference` (read-only diff lens + `--apply-clean-hunks` two-way auto-apply via pure-JS unified-diff parser/applier in `apply-hunks.ts` + `diff-text.ts`), `migrate-fence` (one-shot strip of legacy fence; cumulative-slugs receipt → row-parsing fallback; preserves rows verbatim as user-owned routing), `scrub-legacy-fence-rows` (opt-in row cleanup with skill-present + non-empty-triggers gate), `harvest` (host→gbrain inverse with symlink-reject + canonical-path containment via a `validateUploadPath`-style gate + default-on privacy linter in `harvest-lint.ts` against `~/.gbrain/harvest-private-patterns.txt` plus built-in a built-in fork-name pattern + email + Slack-channel patterns; rollback on match). Paired-source declarations live in each SKILL.md's frontmatter `sources:` array (validated by `loadSkillSources` in `bundle.ts`). `autoDetectSkillsDir` (in `src/core/repo-root.ts`) has a `cwd_walk_up` tier ahead of `~/.openclaw/workspace` (`$OPENCLAW_WORKSPACE` precedence preserved). `gbrain skillpack check --strict` exits non-zero on drift (CI gate); top-level `gbrain skillpack-check` keeps exit-1-on-issues for cron. Companion editorial skill `skills/skillpack-harvest/SKILL.md` drives the genericization checklist. Doc: `docs/guides/skillpacks-as-scaffolding.md`. Test coverage across `test/skillpack-{copy,scaffold,reference,reference-apply,apply-hunks,migrate-fence,scrub-legacy,harvest,harvest-lint,frontmatter-sources}.test.ts` + 9-case E2E in `test/e2e/skillpack-flow.test.ts`. `installer.ts` + `test/skillpack-install.test.ts` survive — `gbrain skillpack diff` still uses `diffSkill` from there.
- `src/core/skillpack/{manifest-v1,tarball,state,remote-source,trust-prompt,bootstrap-display,scaffold-third-party,registry-schema,registry-client,rubric,doctor,init-scaffold,pack-publish,endorse,audit}.ts` + `examples/skillpack-reference/` + `docs/skillpack-anatomy.md` + `scripts/build-skillpack-anatomy.ts` — third-party skillpack ecosystem. `gbrain skillpack scaffold <owner/repo|https-url|./tgz|./local-dir>` resolves the spec via `classifySpec`, fetches through SSRF-hardened `git-remote.ts` (git) or extracts the tarball into `~/.gbrain/skillpack-cache/<host>/<owner>/<repo>/<sha>/`, validates `skillpack.json` (api_version `gbrain-skillpack-v1`), checks `gbrain_min_version`, surfaces a TOFU first-install identity-confirm prompt (author + source + pinned commit + tarball SHA + tier; non-TTY requires `--trust`), records the pin in machine-owned `~/.gbrain/skillpack-state.json` (schema `gbrain-skillpack-state-v1`, atomic `.tmp + rename`, `isAlreadyTrusted` skips re-prompt on author+pin match), runs through `enumerateScaffoldEntries``copyArtifacts` (one-time additive, refuses to overwrite), then DISPLAYS `runbooks/bootstrap.md` WITHOUT executing (deliberately does not auto-execute). Registry catalog at `garrytan/gbrain-skillpack-registry` split into `registry.json` (PR-able, `gbrain-registry-v1`) + `endorsements.json` (Garry-only overlay, `gbrain-endorsements-v1`); `effectiveTier` merges. `registry-client.ts` fetches both via `If-None-Match` etag with 1h soft-TTL + stale-fallback (origins `fresh_fetch | cache_warm | cache_soft_stale | cache_hard_stale`); hard-fail only on no-cache + no-network. CLI: `gbrain skillpack {search,info,registry,doctor,init,pack,endorse}`. Doctor walks `SKILLPACK_RUBRIC_V1` (10 binary dimensions: 5 required CORE — manifest_valid, skills_have_skill_md, routing_evals_present ≥5 intents, skills_have_unique_triggers MECE, changelog_present_and_current — and 5 quality BADGES — unit_tests_present, e2e_tests_present, llm_eval_present ≥3 cases, bootstrap_runbook_present, license_present); tier eligibility: `endorsed` needs all 10, `community` needs core + ≥3 badges, `experimental` needs core only, `blocked` when any core fails. `--quick` ~5s structural sweep; `--fix --yes` auto-scaffolds `auto_fixable: true` dimensions and refuses to overwrite files whose mtime is newer than `skillpack.json`. `gbrain skillpack init <name>` lands 11 files (skillpack.json, SKILL.md, routing-eval.jsonl, test/example.test.ts, e2e/example.e2e.test.ts, evals/example.judge.json, runbooks/{bootstrap,uninstall,upgrade-template}.md, CHANGELOG, README, LICENSE); freshly-init'd scores 10/10; `--minimal` skips test/e2e/evals. `gbrain skillpack pack` packs a deterministic tarball via GNU tar (`--sort=name --mtime=@0 --owner=0 --group=0 --numeric-owner` + `GZIP=-n` + `TZ=UTC`); refuses on `tier_eligibility === 'blocked'`. Extract caps (5000 files / 100MB total / 1MB per file / 255-char paths / 100:1 compression ratio); rejects symlinks/hardlinks/devices/FIFOs. `gbrain skillpack endorse <name> [--tier ...] [--push] [--dry-run]` runs in a clone of the registry repo: validates the pack in `registry.json`, mutates `endorsements.json` via pure `applyEndorsement`, stable-key-orders the write, commits `endorse: <name> -> <tier>`, optionally pushes. JSONL audit at `~/.gbrain/audit/skillpack-YYYY-Www.jsonl` (ISO-week rotated, honors `GBRAIN_AUDIT_DIR`). `examples/skillpack-reference/` is a 10/10 reference pack pinned by `test/e2e/skillpack-third-party.test.ts`. `docs/skillpack-anatomy.md` auto-generated via `scripts/build-skillpack-anatomy.ts` (`--check` for CI drift). CLI dispatch in `src/commands/skillpack.ts` disambiguates third-party (contains `/`, `://`, `.tgz`) from bundled-skill kebab; kebab routes bundled-first, registry-fallback. Tests: `test/skillpack-{manifest-v1,tarball,state,remote-source,trust-prompt,registry-schema,registry-client,rubric,doctor,init-scaffold,pack-publish,endorse,audit,scaffold-third-party}.test.ts` + `test/e2e/skillpack-third-party.test.ts`. Spec at `docs/designs/SKILLPACK_REGISTRY_V1_SPEC.md`.
- `src/core/archive-crawler-config.ts` — safety gate for the `archive-crawler` skill. Refuses to run unless `archive-crawler.scan_paths:` is explicitly set in the brain repo's `gbrain.yml`. Mirrors the storage-config.ts parsing pattern (sibling file, separate concern from storage tiering). `loadArchiveCrawlerConfig(repoPath)` throws `ArchiveCrawlerConfigError(missing_section | empty_scan_paths | invalid_path | parse_error)`. `normalizeAndValidateArchiveCrawlerConfig` rejects relative paths and `..` traversal; `~` is expanded; trailing-slash normalized for unambiguous prefix matching. `isPathAllowed(candidate, config)` is the runtime per-file gate (scan_paths prefix-match with directory-boundary correctness; deny_paths overrides). Pinned by `test/archive-crawler-config.test.ts` (19 cases).
- `src/core/archive-crawler-config.ts` — safety gate for the `archive-crawler` skill. Refuses to run unless `archive-crawler.scan_paths:` is explicitly set in the brain repo's `gbrain.yml`. Mirrors the storage-config.ts parsing pattern (sibling file, separate concern from storage tiering). `loadArchiveCrawlerConfig(repoPath)` throws `ArchiveCrawlerConfigError(missing_section | empty_scan_paths | invalid_path | parse_error)`. `normalizeAndValidateArchiveCrawlerConfig` rejects relative paths and `..` traversal; `~` is expanded; paths are stored resolved and terminated with the PLATFORM separator (`path.sep`) so error output reads natively on each OS. `isPathAllowed(candidate, config)` is the runtime per-file gate (scan_paths prefix-match with directory-boundary correctness; deny_paths overrides). Candidate, scan_paths and deny_paths all funnel through the private `toComparablePrefix()` before the prefix test — on Windows it folds `\``/` and lowercases (NTFS is case-insensitive, so a deny_path spelled `Private` must still match `private`, else the gate fails OPEN); on POSIX it is identity apart from the trailing separator, deliberately NOT folding, since `\` is a legal filename character and paths are case-sensitive. Storing a native separator while appending a hardcoded `/` is the mixed-separator bug that made `isPathAllowed` deny every real path on Windows; the two functions must stay symmetric or the prefix test is meaningless. Pinned by `test/archive-crawler-config.test.ts` (26 cases, platform-selected fixtures + `it.if`-gated win32/POSIX comparison semantics).
- `test/helpers/cli-pty-runner.ts` — generic real-PTY harness (~470 lines) using pure `Bun.spawn({terminal:})` (Bun 1.3.10+; engines.bun pin in package.json). Generic primitives only, no plan-mode orchestrators. Exports `launchPty`, `resolveBinary`, `stripAnsi`, `parseNumberedOptions`, `optionsSignature`, `isNumberedOptionListVisible`, `isTrustDialogVisible`. Self-tests in `test/cli-pty-runner.test.ts` (24 cases).
- `src/core/skillpack/{init-brain-pack,brain-pack-advisory,brain-pack-lint,brain-resident-locate,nag-state}.ts` (#2180) — brain-resident skillpacks. `manifest-v1.ts` gains optional `brain_resident` + `schema_pack` (additive). `runInitBrainPack` scaffolds a pack beside brain content (`brain_resident:true`, exact `gbrain_min_version`, 5-section machine-parseable README); `applyWritePlan` is factored out of `init-scaffold.ts` for the shared refuse-overwrite loop. `brain-pack-lint.lintBrainPackTools` validates each skill's `tools:` against the serving op set (E6 version-skew). Topology A: `src/commands/sources.ts` `runAdd` prints `brain-pack-advisory` to stderr after `opsAddSource`, fail-open; `nag-state.ts` (`~/.gbrain/skillpack-nag-state.json`) keys declines by `(source-repo brain_id, source_id, pack_name)` with pure `decideNagAction` (first/reminder/version-bump/ceiling) — declines count ONLY on CLI-interactive displays. Topology B: `brain-resident-locate.loadResidentPacksForServer` (source-scoped via `sourceScopeOpts`) backs the `list_brain_skillpack` op; `getResidentSkillDetail` backs `get_skill` `source_id`; `scaffold_spec` is the git source, never a server FS path. Tests: `test/skillpack-{init-brain-pack,nag-state,brain-resident-locate}.test.ts` + the brain-resident cases in `test/skillpack-manifest-v1.test.ts`.
- `src/core/advisor/{types,run,render,recommended-set,history,apply,collect-*}.ts` + `src/commands/advisor.ts` (#2180) — `gbrain advisor`: read-only ranked actions from brain state. `run.runAdvisor` executes 8 hardcoded collectors (version [cache-only], migration, schema-pack, stalled-jobs [absent-table tolerant], usage-shape, setup-smells, uninstalled-brain-pack, uninstalled-bundled), each in its own try/catch; `rankFindings` orders critical>warn>info then collector order, caps the info tail, and drops `workspace_dependent` findings when `remote` (A1). `render.ts` is the shared `=`-bar renderer used by the advisor AND `post-install-advisory.ts` (generalized to a single current-state `recommended-set.RECOMMENDED`, `install``scaffold`). `history.ts` appends bounded `~/.gbrain/advisor-history.jsonl` (no DB migration) for since-last-run deltas; local-only. `apply.resolveApplyTarget` is the allowlist+injection guard for `commands/advisor.ts --apply <id>` (structured argv, never a shell; local-only). The `advisor` op (`operations.ts`) is read-scoped, NOT localOnly, gated by `mcp.publish_advisor` (config.ts; default off) and strictly read-only on remote. CLI wired in `cli.ts` (`CLI_ONLY` + dispatch). Bundled skill `skills/gbrain-advisor/` + weekly cron recipe. Tests: `test/advisor-{core,apply,op-gate,ranking-eval}.test.ts`.
+75 -14
View File
@@ -29,11 +29,12 @@
import { existsSync, readFileSync } from 'fs';
import { homedir } from 'os';
import { isAbsolute, join, resolve as resolvePath } from 'path';
import { isAbsolute, join, sep, resolve as resolvePath } from 'path';
export interface ArchiveCrawlerConfig {
/** Absolute paths the agent is permitted to scan. ~ expanded; paths
* normalized to absolute form; trailing-slash normalized.
* normalized to absolute form; terminated with the PLATFORM separator
* (`\` on Windows, `/` on POSIX).
* Required to be non-empty when the section exists. */
scan_paths: string[];
/** Absolute paths within scan_paths to explicitly deny. Optional;
@@ -134,6 +135,48 @@ function expandHome(p: string): string {
return p;
}
const IS_WINDOWS = process.platform === 'win32';
/**
* toComparablePrefix — the ONE canonical form used for every prefix
* comparison in this module. Both the stored allow/deny paths and the
* candidate path must go through this, or the prefix test is
* meaningless.
*
* Stored paths keep native separators (see normalizeOnePath) so error
* messages and CLI output read naturally on each platform; this
* function exists so the *comparison* is separator- and case-agnostic
* without the stored form having to be.
*
* On Windows:
* - `\` is folded to `/`. `resolve()` emits `\`, but a user may
* legitimately write `C:/Users/...` in gbrain.yml (Win32 accepts
* both), and a caller may hand-build a config with either. Mixing
* the two at the boundary is precisely the bug this replaces: a
* `\`-joined candidate never matched a `/`-terminated prefix, so
* isPathAllowed denied every real path.
* - The result is lowercased. NTFS is case-insensitive, so
* `...\writing\Private\` and `...\writing\private\` are the SAME
* directory. A case-sensitive compare would let
* `...\writing\private\tax.md` slip past a deny_path spelled
* `Private` — a fail-OPEN against exactly the sensitive content
* D12 exists to fence off.
*
* On POSIX: identity apart from the trailing separator. Deliberately
* NOT folded — `\` is a legal filename character and paths are
* case-sensitive, so folding either would collide two genuinely
* different paths into one comparable and fail open.
*
* The trailing `/` is appended AFTER folding so a single check covers
* both a native-separator tail (`C:\a\b\`) and an already-forward
* -slashed one, keeping directory-boundary matching intact
* (`/a/b/` must not match `/a/bc/`).
*/
function toComparablePrefix(p: string): string {
const folded = IS_WINDOWS ? p.replace(/\\/g, '/').toLowerCase() : p;
return folded.endsWith('/') ? folded : folded + '/';
}
/**
* Normalize and validate a parsed RawArchiveCrawler into the public
* ArchiveCrawlerConfig shape.
@@ -147,9 +190,11 @@ function expandHome(p: string): string {
* - Path-traversal rejection: a path containing `..` after
* normalization is rejected to prevent allow-list escape via
* `~/Documents/../../../etc/passwd`. Throws invalid_path.
* - Trailing slash normalization: paths without trailing slash get
* one appended (so prefix matching is unambiguous: `/a/b/`
* does NOT match `/a/bc/`).
* - Trailing separator normalization: paths without a trailing
* separator get the PLATFORM separator appended (so prefix matching
* is unambiguous: `/a/b/` does NOT match `/a/bc/`). Separator- and
* case-folding for the comparison itself lives in
* toComparablePrefix(), which isPathAllowed() applies to both sides.
*/
export function normalizeAndValidateArchiveCrawlerConfig(
raw: RawArchiveCrawler,
@@ -194,10 +239,16 @@ function normalizeOnePath(raw: string, field: 'scan_paths' | 'deny_paths'): stri
);
}
// Normalize: resolve any tail and ensure trailing slash for unambiguous
// prefix-matching. resolve() strips trailing slash; we re-add it.
// Normalize: resolve any tail and ensure a trailing separator for
// unambiguous prefix-matching. resolve() strips the trailing separator;
// we re-add it using the PLATFORM separator, not a hardcoded '/' —
// resolve() emits '\' on Windows, so appending '/' produced a
// mixed-separator path ('C:\Users\...\writing/') that no candidate
// could ever prefix-match. Comparison is done on the folded form from
// toComparablePrefix(), so storing native separators here is safe and
// keeps error messages readable on each platform.
const resolved = resolvePath(expanded);
return resolved.endsWith('/') ? resolved : resolved + '/';
return resolved.endsWith(sep) ? resolved : resolved + sep;
}
/**
@@ -267,8 +318,15 @@ export function loadArchiveCrawlerConfig(
* (when it grows a runtime check) to gate per-file decisions.
*
* Both inputs are normalized via `resolvePath` and compared as absolute
* directory prefixes (with trailing slash) so `media/x/` does not match
* `media/xerox/foo`.
* directory prefixes (with trailing separator) so `media/x/` does not
* match `media/xerox/foo`.
*
* Every side of the comparison — candidate, scan_paths and deny_paths —
* is funnelled through toComparablePrefix() so the three agree on
* separator and case. The config entries are folded here rather than
* trusted as-is because isPathAllowed is part of the public surface and
* a caller may hand-build an ArchiveCrawlerConfig without going through
* normalizeAndValidateArchiveCrawlerConfig().
*/
export function isPathAllowed(
candidate: string,
@@ -276,14 +334,17 @@ export function isPathAllowed(
): boolean {
const expanded = expandHome(candidate);
if (!isAbsolute(expanded)) return false;
const resolved = resolvePath(expanded);
const prefix = resolved.endsWith('/') ? resolved : resolved + '/';
const prefix = toComparablePrefix(resolvePath(expanded));
// Must be inside at least one scan_path.
const allowed = config.scan_paths.some((sp) => prefix.startsWith(sp));
const allowed = config.scan_paths.some((sp) =>
prefix.startsWith(toComparablePrefix(sp)),
);
if (!allowed) return false;
// Must NOT be inside any deny_path.
const denied = config.deny_paths.some((dp) => prefix.startsWith(dp));
const denied = config.deny_paths.some((dp) =>
prefix.startsWith(toComparablePrefix(dp)),
);
return !denied;
}
+5 -2
View File
@@ -15,7 +15,7 @@
* gets a chance to copy, or nothing does.
*/
import { existsSync, lstatSync, mkdirSync, readdirSync, readFileSync, realpathSync, writeFileSync } from 'fs';
import { dirname, join, relative } from 'path';
import { dirname, join, relative, sep } from 'path';
export interface CopyItem {
/** Absolute source path. */
@@ -153,7 +153,10 @@ export function copyArtifacts(items: CopyItem[], opts: CopyArtifactsOpts = {}):
const real = realpathSync(item.source);
// realpathSync returns paths without trailing slash; add path
// separator to the prefix check so /a/b doesn't match /a/bb.
const prefix = confineRoot.endsWith('/') ? confineRoot : confineRoot + '/';
// Both sides are realpathSync() output, so the separator must be the
// NATIVE one — a hardcoded '/' never matches a win32 path and rejected
// every source as path_traversal.
const prefix = confineRoot.endsWith(sep) ? confineRoot : confineRoot + sep;
if (real !== confineRoot && !real.startsWith(prefix)) {
throw new CopyError(
`${item.source}: path traversal rejected. Source canonicalizes outside the confinement root (${confineRoot}).`,
+146 -42
View File
@@ -9,15 +9,27 @@
* - empty scan_paths -> empty_scan_paths
* - relative path -> invalid_path
* - path traversal (..) -> invalid_path
* - valid config -> normalized absolute trailing-slashed paths
* - valid config -> normalized absolute trailing-separator paths
* - ~ expansion
* - deny_paths optional
* - isPathAllowed: prefix match + deny override + prefix boundary
*
* PLATFORM NOTE: these tests must run on both POSIX (gbrain CI is
* 100% ubuntu-latest) and Windows. POSIX path literals cannot be
* shared across both, because `/home/user` is NOT a drive-qualified
* absolute path on Windows — `path.resolve('/home/user')` returns
* `C:\home\user` (the cwd's drive). That is correct Win32 semantics,
* not a product bug, so the FIXTURES are platform-selected via ROOT
* rather than the product being forced to emit POSIX paths.
*
* Only the separator character is taken from `path`; the directory
* structure in every expectation is still hand-written, so these stay
* real assertions rather than a tautological re-run of the impl.
*/
import { describe, expect, it, beforeEach, afterEach } from 'bun:test';
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs';
import { join } from 'path';
import { mkdtempSync, rmSync, writeFileSync } from 'fs';
import { join, sep } from 'path';
import { homedir, tmpdir } from 'os';
import {
loadArchiveCrawlerConfig,
@@ -26,6 +38,14 @@ import {
ArchiveCrawlerConfigError,
} from '../src/core/archive-crawler-config.ts';
const WIN = process.platform === 'win32';
/** Absolute-path fixture root for the current platform. */
const ROOT = WIN ? 'C:\\gbtest' : '/home/user';
/** Shorthand for the platform separator used in expectations. */
const S = sep;
let workdir: string;
beforeEach(() => {
@@ -79,7 +99,7 @@ describe('loadArchiveCrawlerConfig — D12 missing_section', () => {
describe('loadArchiveCrawlerConfig — D12 empty_scan_paths', () => {
it('throws empty_scan_paths when scan_paths is omitted', () => {
writeYaml('archive-crawler:\n deny_paths:\n - /tmp/forbidden/\n');
writeYaml(`archive-crawler:\n deny_paths:\n - ${join(ROOT, 'forbidden')}\n`);
expect(() => loadArchiveCrawlerConfig(workdir)).toThrow(ArchiveCrawlerConfigError);
try {
loadArchiveCrawlerConfig(workdir);
@@ -106,7 +126,9 @@ describe('loadArchiveCrawlerConfig — D12 invalid_path', () => {
});
it('throws invalid_path on path traversal (..)', () => {
writeYaml('archive-crawler:\n scan_paths:\n - /home/user/Documents/../../etc/passwd\n');
writeYaml(
`archive-crawler:\n scan_paths:\n - ${join(ROOT, 'Documents')}${S}..${S}..${S}etc${S}passwd\n`,
);
try {
loadArchiveCrawlerConfig(workdir);
throw new Error('expected throw');
@@ -118,9 +140,9 @@ describe('loadArchiveCrawlerConfig — D12 invalid_path', () => {
it('rejects ".." in deny_paths too', () => {
writeYaml(`archive-crawler:
scan_paths:
- /home/user/Documents/
- ${join(ROOT, 'Documents')}
deny_paths:
- /home/user/Documents/../etc
- ${join(ROOT, 'Documents')}${S}..${S}etc
`);
try {
loadArchiveCrawlerConfig(workdir);
@@ -132,16 +154,16 @@ describe('loadArchiveCrawlerConfig — D12 invalid_path', () => {
});
describe('loadArchiveCrawlerConfig — happy path', () => {
it('returns normalized absolute paths with trailing slash', () => {
it('returns normalized absolute paths with trailing separator', () => {
writeYaml(`archive-crawler:
scan_paths:
- /home/user/writing
- /mnt/backup/old-letters/
- ${join(ROOT, 'writing')}
- ${join(ROOT, 'backup', 'old-letters')}${S}
`);
const config = loadArchiveCrawlerConfig(workdir);
expect(config.scan_paths).toEqual([
'/home/user/writing/',
'/mnt/backup/old-letters/',
`${ROOT}${S}writing${S}`,
`${ROOT}${S}backup${S}old-letters${S}`,
]);
expect(config.deny_paths).toEqual([]);
});
@@ -150,28 +172,28 @@ describe('loadArchiveCrawlerConfig — happy path', () => {
const home = homedir();
writeYaml('archive-crawler:\n scan_paths:\n - ~/Documents/writing\n');
const config = loadArchiveCrawlerConfig(workdir);
expect(config.scan_paths[0]).toBe(`${home}/Documents/writing/`);
expect(config.scan_paths[0]).toBe(`${home}${S}Documents${S}writing${S}`);
});
it('accepts deny_paths alongside scan_paths', () => {
writeYaml(`archive-crawler:
scan_paths:
- /home/user/Documents/
- ${join(ROOT, 'Documents')}${S}
deny_paths:
- /home/user/Documents/finances/
- /home/user/Documents/medical/
- ${join(ROOT, 'Documents', 'finances')}${S}
- ${join(ROOT, 'Documents', 'medical')}${S}
`);
const config = loadArchiveCrawlerConfig(workdir);
expect(config.deny_paths).toEqual([
'/home/user/Documents/finances/',
'/home/user/Documents/medical/',
`${ROOT}${S}Documents${S}finances${S}`,
`${ROOT}${S}Documents${S}medical${S}`,
]);
});
it('accepts both archive-crawler and archive_crawler key spellings', () => {
writeYaml('archive_crawler:\n scan_paths:\n - /home/user/notes\n');
writeYaml(`archive_crawler:\n scan_paths:\n - ${join(ROOT, 'notes')}\n`);
const config = loadArchiveCrawlerConfig(workdir);
expect(config.scan_paths[0]).toBe('/home/user/notes/');
expect(config.scan_paths[0]).toBe(`${ROOT}${S}notes${S}`);
});
});
@@ -182,43 +204,125 @@ describe('normalizeAndValidateArchiveCrawlerConfig — direct API', () => {
);
});
it('returns trailing-slashed normalized paths', () => {
it('returns trailing-separator normalized paths', () => {
const out = normalizeAndValidateArchiveCrawlerConfig({
scan_paths: ['/a/b', '/c/d/'],
scan_paths: [join(ROOT, 'a', 'b'), `${join(ROOT, 'c', 'd')}${S}`],
});
expect(out.scan_paths).toEqual(['/a/b/', '/c/d/']);
expect(out.scan_paths).toEqual([
`${ROOT}${S}a${S}b${S}`,
`${ROOT}${S}c${S}d${S}`,
]);
});
it('terminates every normalized path with the PLATFORM separator, not a foreign one', () => {
// Regression pin for the mixed-separator bug: resolve() emits '\' on
// Windows but the old code appended a hardcoded '/', producing
// 'C:\Users\...\writing/'. isPathAllowed then compared a '\'-joined
// candidate against that '/'-terminated prefix and never matched.
const out = normalizeAndValidateArchiveCrawlerConfig({
scan_paths: [join(ROOT, 'writing')],
deny_paths: [join(ROOT, 'writing', 'private')],
});
for (const p of [...out.scan_paths, ...out.deny_paths]) {
expect(p.endsWith(S)).toBe(true);
if (WIN) {
// No stray forward slash anywhere: the whole path is native-separator.
expect(p.includes('/')).toBe(false);
}
}
});
});
describe('isPathAllowed', () => {
const config = {
scan_paths: ['/home/user/writing/', '/home/user/Dropbox/'],
deny_paths: ['/home/user/Dropbox/finances/'],
};
it('returns true for a path inside a scan_path', () => {
expect(isPathAllowed('/home/user/writing/essay.md', config)).toBe(true);
expect(isPathAllowed('/home/user/Dropbox/letters/a.txt', config)).toBe(true);
describe('isPathAllowed — round-trip against the real normalizer', () => {
// This is the case the original suite lacked: it hand-built the config
// from POSIX literals instead of feeding it through
// normalizeAndValidateArchiveCrawlerConfig, so the separator mismatch
// between the two functions was invisible. Four of the five old
// assertions also expected `false`, which the Windows total-deny
// satisfied by accident.
const config = normalizeAndValidateArchiveCrawlerConfig({
scan_paths: [join(ROOT, 'writing'), join(ROOT, 'Dropbox')],
deny_paths: [join(ROOT, 'Dropbox', 'finances')],
});
it('returns false for a path outside any scan_path', () => {
expect(isPathAllowed('/etc/passwd', config)).toBe(false);
expect(isPathAllowed('/home/user/Other/thing.md', config)).toBe(false);
it('allows a file inside a scan_path', () => {
expect(isPathAllowed(join(ROOT, 'writing', 'essay.md'), config)).toBe(true);
expect(isPathAllowed(join(ROOT, 'Dropbox', 'letters', 'a.txt'), config)).toBe(true);
});
it('returns false for a path inside a deny_path even if it is also in a scan_path', () => {
expect(isPathAllowed('/home/user/Dropbox/finances/2024.pdf', config)).toBe(false);
it('allows the scan_path directory itself', () => {
expect(isPathAllowed(join(ROOT, 'writing'), config)).toBe(true);
});
it('denies a file outside every scan_path', () => {
expect(isPathAllowed(join(ROOT, 'Other', 'thing.md'), config)).toBe(false);
});
it('denies a file inside a deny_path even though it is also in a scan_path', () => {
expect(isPathAllowed(join(ROOT, 'Dropbox', 'finances', '2024.pdf'), config)).toBe(false);
});
it('respects directory boundaries — /writing/ does not match /writing-stuff/', () => {
// Exact-prefix-with-trailing-slash means /home/user/writing/ does NOT
// match /home/user/writing-stuff/. This is the codex T7 / storage-config
// pattern: prefix matching at directory boundaries, not arbitrary string
// prefixes.
expect(isPathAllowed('/home/user/writing-stuff/file.md', config)).toBe(false);
// Exact-prefix-with-trailing-separator means .../writing/ does NOT
// match .../writing-stuff/. This is the codex T7 / storage-config
// pattern: prefix matching at directory boundaries, not arbitrary
// string prefixes.
expect(isPathAllowed(join(ROOT, 'writing-stuff', 'file.md'), config)).toBe(false);
});
it('collapses traversal in the candidate before matching', () => {
// resolve() flattens '..', so an escape attempt lands outside the
// scan_path and is denied on its resolved form.
expect(isPathAllowed(join(ROOT, 'writing', '..', 'Other', 'x.md'), config)).toBe(false);
});
it('rejects relative paths', () => {
expect(isPathAllowed('./relative.md', config)).toBe(false);
});
});
describe('isPathAllowed — platform-specific comparison semantics', () => {
it.if(WIN)('win32: matches forward-slash input against native-separator config', () => {
const config = normalizeAndValidateArchiveCrawlerConfig({
scan_paths: [join(ROOT, 'writing')],
});
// A user may legitimately write forward slashes in gbrain.yml on
// Windows; Win32 accepts them. Comparison must not care.
expect(isPathAllowed('C:/gbtest/writing/essay.md', config)).toBe(true);
});
it.if(WIN)('win32: comparison is case-insensitive (NTFS semantics)', () => {
const config = normalizeAndValidateArchiveCrawlerConfig({
scan_paths: [join(ROOT, 'writing')],
deny_paths: [join(ROOT, 'writing', 'Private')],
});
// Drive letter case must not matter.
expect(isPathAllowed('c:\\gbtest\\writing\\essay.md', config)).toBe(true);
// And neither may the deny_path's case — this is the fail-OPEN case:
// 'Private' and 'private' are the SAME directory on NTFS, so a
// case-sensitive compare would have let the sensitive file through.
expect(isPathAllowed(join(ROOT, 'writing', 'private', 'tax.md'), config)).toBe(false);
expect(isPathAllowed(join(ROOT, 'writing', 'PRIVATE', 'tax.md'), config)).toBe(false);
});
it.if(!WIN)('posix: comparison stays case-sensitive', () => {
// Negative control for the win32 case-folding: POSIX filesystems are
// case-sensitive, so folding there would be a real fail-open.
const config = normalizeAndValidateArchiveCrawlerConfig({
scan_paths: ['/home/user/Writing'],
});
expect(isPathAllowed('/home/user/Writing/essay.md', config)).toBe(true);
expect(isPathAllowed('/home/user/writing/essay.md', config)).toBe(false);
});
it.if(!WIN)('posix: a literal backslash in a filename is not treated as a separator', () => {
// Backslash is a legal POSIX filename character. Folding '\' -> '/'
// on POSIX would collide two genuinely different paths, so the
// comparator must leave POSIX paths untouched.
const config = normalizeAndValidateArchiveCrawlerConfig({
scan_paths: ['/home/user/a\\b'],
});
expect(isPathAllowed('/home/user/a\\b/file.md', config)).toBe(true);
expect(isPathAllowed('/home/user/a/b/file.md', config)).toBe(false);
});
});
+11 -2
View File
@@ -3,12 +3,21 @@
// so the cross-discriminator surface stays MECE.
import { describe, test, expect } from 'bun:test';
import { sep } from 'path';
import {
detectArtifactKind,
targetDirForKind,
validateManifestByKind,
} from '../src/core/artifact/index.ts';
// targetDirForKind builds its result with path.join(), which is correct:
// it emits native separators on every platform. The fixture below is
// therefore platform-selected rather than a shared POSIX literal — on
// Windows, join('/home/u/.gbrain', 'skillpacks') is '\home\u\.gbrain\
// skillpacks', which is right, not a bug. Only the separator comes from
// `path`; the subdirectory names being asserted are still hand-written.
const GBRAIN_HOME = process.platform === 'win32' ? 'C:\\gb' : '/home/u/.gbrain';
describe('v0.39 T14 — artifact abstraction', () => {
test('detectArtifactKind by extension', () => {
expect(detectArtifactKind('/tmp/foo.gbrain-schema')).toBe('schemapack');
@@ -17,8 +26,8 @@ describe('v0.39 T14 — artifact abstraction', () => {
});
test('targetDirForKind routes to distinct subdirectories', () => {
expect(targetDirForKind('schemapack', '/home/u/.gbrain')).toBe('/home/u/.gbrain/schema-packs');
expect(targetDirForKind('skillpack', '/home/u/.gbrain')).toBe('/home/u/.gbrain/skillpacks');
expect(targetDirForKind('schemapack', GBRAIN_HOME)).toBe(`${GBRAIN_HOME}${sep}schema-packs`);
expect(targetDirForKind('skillpack', GBRAIN_HOME)).toBe(`${GBRAIN_HOME}${sep}skillpacks`);
});
test('validateManifestByKind: schemapack happy path', () => {
+4 -2
View File
@@ -1,6 +1,6 @@
import { describe, test, expect, afterEach } from 'bun:test';
import { mkdtempSync, writeFileSync, rmSync } from 'fs';
import { join } from 'path';
import { join, isAbsolute } from 'path';
import { tmpdir } from 'os';
import {
loadMounts,
@@ -124,7 +124,9 @@ describe('loadMounts — entry validation', () => {
mounts: [{ id: 'a', path: '/tmp/relative-test', engine: 'pglite', database_path: '/tmp/a/.pg' }],
}));
const mounts = loadMounts(path);
expect(mounts[0].path.startsWith('/')).toBe(true);
// loadMounts resolve()s the path: 'C:\…' on win32, so a leading-'/'
// check is the wrong absoluteness test.
expect(isAbsolute(mounts[0].path)).toBe(true);
});
test('enabled=false is preserved', () => {
+36 -27
View File
@@ -20,11 +20,20 @@
import { describe, expect, test, beforeAll, afterAll } from 'bun:test';
import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'fs';
import { execFileSync } from 'child_process';
import { join } from 'path';
import { join, sep } from 'path';
import { tmpdir } from 'os';
import { scanBrainSources, walkDir } from '../src/core/brain-writer.ts';
import { collectFiles } from '../src/commands/frontmatter.ts';
/**
* Build a separator-prefixed path fragment for suffix/substring predicates.
* The walkers emit native-separator paths, so a hardcoded '/' fragment makes
* the positive assertions fail on Windows AND — worse — makes every negative
* `toBe(false)` guard pass vacuously, proving nothing. Byte-identical to the
* old '/foo/bar' literals on POSIX.
*/
const seg = (...parts: string[]) => sep + join(...parts);
let root: string;
beforeAll(() => {
@@ -64,19 +73,19 @@ describe('walkDir (brain-writer.ts) — descent-time pruning', () => {
test('does NOT descend into node_modules at any depth', () => {
const visited: string[] = [];
walkDir(root, () => {}, (dir) => visited.push(dir));
expect(visited.some(d => d.includes('/node_modules'))).toBe(false);
expect(visited.some(d => d.includes(seg('node_modules')))).toBe(false);
});
test('does NOT descend into .git', () => {
const visited: string[] = [];
walkDir(root, () => {}, (dir) => visited.push(dir));
expect(visited.some(d => d.endsWith('/.git') || d.includes('/.git/'))).toBe(false);
expect(visited.some(d => d.endsWith(seg('.git')) || d.includes(seg('.git') + sep))).toBe(false);
});
test('does NOT descend into .obsidian (dot-prefix heuristic)', () => {
const visited: string[] = [];
walkDir(root, () => {}, (dir) => visited.push(dir));
expect(visited.some(d => d.includes('/.obsidian'))).toBe(false);
expect(visited.some(d => d.includes(seg('.obsidian')))).toBe(false);
});
test('does NOT descend into *.raw sidecar dirs', () => {
@@ -88,22 +97,22 @@ describe('walkDir (brain-writer.ts) — descent-time pruning', () => {
test('does NOT descend into git submodule directories (.git as FILE)', () => {
const visited: string[] = [];
walkDir(root, () => {}, (dir) => visited.push(dir));
expect(visited.some(d => d.endsWith('/people/submod'))).toBe(false);
expect(visited.some(d => d.endsWith(seg('people', 'submod')))).toBe(false);
});
test('DOES descend into regular subdirs and visits .md files there', () => {
const visited: string[] = [];
const files: string[] = [];
walkDir(root, (f) => { files.push(f); }, (dir) => visited.push(dir));
expect(visited.some(d => d.endsWith('/people'))).toBe(true);
expect(visited.some(d => d.endsWith('/concepts/subdir'))).toBe(true);
expect(visited.some(d => d.endsWith(seg('people')))).toBe(true);
expect(visited.some(d => d.endsWith(seg('concepts', 'subdir')))).toBe(true);
// ops/ is ordinary content — descended, not pruned (#2404).
expect(visited.some(d => d.endsWith('/ops/logs'))).toBe(true);
expect(files.some(f => f.endsWith('/people/alice.md'))).toBe(true);
expect(files.some(f => f.endsWith('/concepts/subdir/thing.md'))).toBe(true);
expect(files.some(f => f.endsWith('/ops/logs/run.md'))).toBe(true);
expect(visited.some(d => d.endsWith(seg('ops', 'logs')))).toBe(true);
expect(files.some(f => f.endsWith(seg('people', 'alice.md')))).toBe(true);
expect(files.some(f => f.endsWith(seg('concepts', 'subdir', 'thing.md')))).toBe(true);
expect(files.some(f => f.endsWith(seg('ops', 'logs', 'run.md')))).toBe(true);
// And explicitly does NOT visit the file under node_modules.
expect(files.some(f => f.includes('/node_modules/'))).toBe(false);
expect(files.some(f => f.includes(seg('node_modules') + sep))).toBe(false);
});
test('regression: pre-v0.38.2.0 walker would have descended into node_modules and stat\'d every entry', () => {
@@ -112,7 +121,7 @@ describe('walkDir (brain-writer.ts) — descent-time pruning', () => {
// visitDir would be called with node_modules paths.
const descents: string[] = [];
walkDir(root, () => {}, (d) => descents.push(d));
const vendor = descents.filter(d => /\/(node_modules|\.git|\.obsidian)(\/|$)/.test(d) || /\.raw$/.test(d));
const vendor = descents.filter(d => /[\\/](node_modules|\.git|\.obsidian)([\\/]|$)/.test(d) || /\.raw$/.test(d));
expect(vendor).toEqual([]);
});
});
@@ -121,38 +130,38 @@ describe('collectFiles (frontmatter.ts) — descent-time pruning parity', () =>
test('does NOT descend into node_modules at any depth', () => {
const visited: string[] = [];
collectFiles(root, (dir) => visited.push(dir));
expect(visited.some(d => d.includes('/node_modules'))).toBe(false);
expect(visited.some(d => d.includes(seg('node_modules')))).toBe(false);
});
test('does NOT descend into .git, .obsidian, or *.raw', () => {
const visited: string[] = [];
collectFiles(root, (dir) => visited.push(dir));
expect(visited.some(d => d.includes('/.git'))).toBe(false);
expect(visited.some(d => d.includes('/.obsidian'))).toBe(false);
expect(visited.some(d => d.includes(seg('.git')))).toBe(false);
expect(visited.some(d => d.includes(seg('.obsidian')))).toBe(false);
expect(visited.some(d => d.endsWith('.raw'))).toBe(false);
});
test('DOES descend into ops/ — ordinary content, not a vendor tree (#2404)', () => {
const visited: string[] = [];
collectFiles(root, (dir) => visited.push(dir));
expect(visited.some(d => d.endsWith('/ops') || d.includes('/ops/'))).toBe(true);
expect(visited.some(d => d.endsWith(seg('ops')) || d.includes(seg('ops') + sep))).toBe(true);
const files = collectFiles(root);
expect(files.some(f => f.endsWith('/ops/logs/run.md'))).toBe(true);
expect(files.some(f => f.endsWith(seg('ops', 'logs', 'run.md')))).toBe(true);
});
test('does NOT descend into git submodule directories', () => {
const visited: string[] = [];
collectFiles(root, (dir) => visited.push(dir));
expect(visited.some(d => d.endsWith('/people/submod'))).toBe(false);
expect(visited.some(d => d.endsWith(seg('people', 'submod')))).toBe(false);
});
test('DOES collect .md files under regular subdirs', () => {
const files = collectFiles(root);
expect(files.some(f => f.endsWith('/people/alice.md'))).toBe(true);
expect(files.some(f => f.endsWith('/concepts/subdir/thing.md'))).toBe(true);
expect(files.some(f => f.includes('/node_modules/'))).toBe(false);
expect(files.some(f => f.includes('/.git/'))).toBe(false);
expect(files.some(f => f.includes('.raw/'))).toBe(false);
expect(files.some(f => f.endsWith(seg('people', 'alice.md')))).toBe(true);
expect(files.some(f => f.endsWith(seg('concepts', 'subdir', 'thing.md')))).toBe(true);
expect(files.some(f => f.includes(seg('node_modules') + sep))).toBe(false);
expect(files.some(f => f.includes(seg('.git') + sep))).toBe(false);
expect(files.some(f => f.includes('.raw' + sep))).toBe(false);
});
test('single-file target returns that file unchanged (no walk)', () => {
@@ -173,9 +182,9 @@ describe('frontmatter walkers — git-visible file parity', () => {
writeFileSync(join(repo, 'people', 'alice.md'), '---\ntitle: Alice\n---\n\nbody\n');
writeFileSync(join(repo, 'local-skills', 'SKILL.md'), '---\nname: bad\n# malformed frontmatter\n');
const files = collectFiles(repo).map((f) => f.replace(repo + '/', ''));
expect(files).toContain('people/alice.md');
expect(files).not.toContain('local-skills/SKILL.md');
const files = collectFiles(repo).map((f) => f.replace(repo + sep, ''));
expect(files).toContain(join('people', 'alice.md'));
expect(files).not.toContain(join('local-skills', 'SKILL.md'));
} finally {
rmSync(repo, { recursive: true, force: true });
}
+8 -4
View File
@@ -133,7 +133,8 @@ describe('check-resolvable — unit: resolveSkillsDir', () => {
it('resolves relative --skills-dir against cwd', () => {
const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, strict: false, skillsDir: 'skills' });
expect(r.dir).toMatch(/\/skills$/);
// r.dir is join()/resolve()-built, so the separator is '\' on win32.
expect(r.dir).toMatch(/[\\/]skills$/);
expect(r.error).toBeNull();
expect(r.source).toBe('explicit');
});
@@ -155,7 +156,8 @@ describe('check-resolvable — unit: resolveSkillsDir', () => {
const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, strict: false, skillsDir: null });
// Install-path fallback succeeds when test runs inside the gbrain repo.
expect(r.error).toBeNull();
expect(r.dir).toMatch(/\/skills$/);
// r.dir is join()/resolve()-built, so the separator is '\' on win32.
expect(r.dir).toMatch(/[\\/]skills$/);
expect(r.source).toBe('install_path');
} finally {
process.chdir(original);
@@ -172,7 +174,8 @@ describe('check-resolvable — unit: resolveSkillsDir', () => {
// back-compat. See src/core/repo-root.ts.
const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, strict: false, skillsDir: null });
expect(r.error).toBeNull();
expect(r.dir).toMatch(/\/skills$/);
// r.dir is join()/resolve()-built, so the separator is '\' on win32.
expect(r.dir).toMatch(/[\\/]skills$/);
expect(r.source).toBe('cwd_walk_up');
});
@@ -373,7 +376,8 @@ describe('gbrain check-resolvable CLI — integration', () => {
const r = run([]);
expect(r.status === 0 || r.status === 1).toBe(true);
expect(r.stdout).toContain('Auto-detected skills directory');
expect(r.stdout).toContain('/skills');
// The logged path is native-format — '\skills' on win32.
expect(r.stdout).toMatch(/[\\/]skills/);
});
// v0.31.7 D6 regression guard: --fix must refuse install-path fallback.
+6 -1
View File
@@ -24,7 +24,12 @@ describe('schema packPathByName', () => {
for (const name of ['gbrain-base', 'gbrain-recommended', 'gbrain-base-v2']) {
const path = _testHelpers.packPathByName(name);
expect(path).toBeTruthy();
expect(path!.endsWith(`src/core/schema-pack/base/${name}.yaml`)).toBe(true);
// The resolved path is native-format, so it ends with `src\core\...`
// on Windows. Only the separator comes from `path`; the directory
// structure stays hand-written. Byte-identical on POSIX.
expect(path!.endsWith(join('src', 'core', 'schema-pack', 'base', `${name}.yaml`))).toBe(
true,
);
expect(existsSync(path!)).toBe(true);
}
});
+2 -1
View File
@@ -250,7 +250,8 @@ describeE2E('E2E: v0.11.0 orchestrator against live Postgres', () => {
for (const todo of todos) {
expect(todo.type).toBe('cron-handler-needs-host-registration');
expect(todo.status).toBe('pending');
expect(todo.manifest_path).toContain('cron/jobs.json');
// manifest_path is join(scope, 'cron', 'jobs.json') — '\' on win32.
expect(todo.manifest_path).toContain(join('cron', 'jobs.json'));
}
} finally {
restoreHomePath();
+3 -2
View File
@@ -28,7 +28,7 @@
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
import { tmpdir } from 'node:os';
import { mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs';
import { join } from 'node:path';
import { join, sep } from 'node:path';
import { PGLiteEngine } from '../../src/core/pglite-engine.ts';
import { resetPgliteState } from '../helpers/reset-pglite.ts';
import { validateSourceId } from '../../src/core/utils.ts';
@@ -253,7 +253,8 @@ describe('multi-source bug class', () => {
// The two paths must NOT collide.
expect(defaultPath).not.toBe(mediaPath);
expect(mediaPath).toContain('.sources/media-corpus/');
// computePath joins, so the segment separator is '\' on win32.
expect(mediaPath).toContain(join('.sources', 'media-corpus') + sep);
// Actually write to both paths to prove disk separation.
mkdirSync(join(tmpDir, 'people'), { recursive: true });
+3 -1
View File
@@ -262,7 +262,9 @@ describe('resumeFilter', () => {
'/tmp/example-brain/meetings/2026-05-13.md',
'/tmp/example-brain/concepts/a.md',
];
const completed = new Set(['meetings/2026-05-13.md']);
// resumeFilter keys on relative(dir, p), which carries native separators —
// build the completed key with join() so it matches on Windows too.
const completed = new Set([join('meetings', '2026-05-13.md')]);
expect(resumeFilter(all, '/tmp/example-brain', completed)).toEqual([
'/tmp/example-brain/concepts/a.md',
]);
+13 -9
View File
@@ -67,15 +67,19 @@ afterAll(() => {
describe('#2607 — git fast path excludes what incremental sync excludes', () => {
test('tracked files under pruned dirs are NOT collected', () => {
const files = rel(collectSyncableFiles(repo, { strategy: 'markdown' }));
expect(files).toContain('notes/real.md');
expect(files).toContain('ops/tasks.md'); // ordinary content (#2404)
expect(files).not.toContain('.obsidian/plugin-notes.md');
expect(files).not.toContain('vendor/pkg/notes.md');
expect(files).not.toContain('node_modules/dep/CHANGELOG.md');
expect(files).not.toContain('people/pedro.raw/source.md');
// rel() returns relative() output, which carries native separators — build
// every expectation with join() to match. A hardcoded '/' literal would not
// just fail the positives, it would make each not.toContain() below pass
// vacuously on Windows, silently retiring the #2607 guard.
expect(files).toContain(join('notes', 'real.md'));
expect(files).toContain(join('ops', 'tasks.md')); // ordinary content (#2404)
expect(files).not.toContain(join('.obsidian', 'plugin-notes.md'));
expect(files).not.toContain(join('vendor', 'pkg', 'notes.md'));
expect(files).not.toContain(join('node_modules', 'dep', 'CHANGELOG.md'));
expect(files).not.toContain(join('people', 'pedro.raw', 'source.md'));
// Metafiles stay excluded too.
expect(files).not.toContain('README.md');
expect(files).not.toContain('notes/index.md');
expect(files).not.toContain(join('notes', 'index.md'));
});
test('full-sync enumeration agrees with incremental isSyncable for every collected file', () => {
@@ -104,8 +108,8 @@ describe('#2607 — git fast path excludes what incremental sync excludes', () =
includeGitignored: true,
}));
expect(defaultFiles).not.toContain('Meetings/weekly.md');
expect(includeIgnored).toContain('Meetings/weekly.md');
expect(defaultFiles).not.toContain(join('Meetings', 'weekly.md'));
expect(includeIgnored).toContain(join('Meetings', 'weekly.md'));
} finally {
rmSync(ignoredRepo, { recursive: true, force: true });
}
+10 -5
View File
@@ -2,6 +2,7 @@ import { describe, test, expect, beforeAll, beforeEach, afterEach } from 'bun:te
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs';
import { tmpdir } from 'os';
import { join } from 'path';
import { resolve, sep } from 'node:path';
import {
parseRecipe,
isUnsafeHealthCheck,
@@ -14,6 +15,8 @@ import {
isInternalUrl,
} from '../src/commands/integrations.ts';
const RECIPES_DIR = resolve(import.meta.dir, '..', 'recipes');
// --- parseRecipe tests ---
describe('parseRecipe', () => {
@@ -259,7 +262,7 @@ describe('twilio-voice-brain recipe', () => {
);
const recipe = parseRecipe(content, 'twilio-voice-brain.md');
expect(recipe).not.toBeNull();
const recipesDir = new URL('../recipes/', import.meta.url).pathname;
const recipesDir = RECIPES_DIR;
for (const dep of recipe!.frontmatter.requires) {
const depPath = resolve(recipesDir, `${dep}.md`);
expect(existsSync(depPath)).toBe(true);
@@ -302,7 +305,7 @@ describe('all recipes', () => {
test('every recipe file in recipes/ parses correctly', () => {
const { readFileSync, readdirSync } = require('fs');
const { resolve } = require('path');
const recipesDir = new URL('../recipes/', import.meta.url).pathname;
const recipesDir = RECIPES_DIR;
const files = readdirSync(recipesDir).filter((f: string) => f.endsWith('.md'));
expect(files.length).toBeGreaterThan(0);
for (const file of files) {
@@ -316,7 +319,7 @@ describe('all recipes', () => {
test('no recipe contains personal references', () => {
const { readFileSync, readdirSync } = require('fs');
const { resolve } = require('path');
const recipesDir = new URL('../recipes/', import.meta.url).pathname;
const recipesDir = RECIPES_DIR;
const files = readdirSync(recipesDir).filter((f: string) => f.endsWith('.md'));
const personalPatterns = /wintermute|mercury|16507969501|\+1650796/i;
for (const file of files) {
@@ -328,7 +331,7 @@ describe('all recipes', () => {
test('typed health_checks parse correctly in all recipes', () => {
const { readFileSync, readdirSync } = require('fs');
const { resolve } = require('path');
const recipesDir = new URL('../recipes/', import.meta.url).pathname;
const recipesDir = RECIPES_DIR;
const files = readdirSync(recipesDir).filter((f: string) => f.endsWith('.md'));
for (const file of files) {
const content = readFileSync(resolve(recipesDir, file), 'utf-8');
@@ -665,7 +668,9 @@ describe('getRecipeDirs (B1 trust boundary)', () => {
expect(typeof d.dir).toBe('string');
}
// In this repo, the source recipes dir must be trusted
const source = dirs.find(d => d.dir.endsWith('/recipes') && d.trusted);
// `dir` is native-format (`...\recipes` on Windows). Only the separator
// comes from `path`; the directory name stays hand-written.
const source = dirs.find(d => d.dir.endsWith(`${sep}recipes`) && d.trusted);
expect(source).toBeDefined();
});
+7 -5
View File
@@ -12,7 +12,7 @@
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
import { mkdtempSync, rmSync, mkdirSync, writeFileSync, readFileSync, existsSync, symlinkSync } from 'fs';
import { join } from 'path';
import { join, sep, dirname } from 'path';
import { tmpdir } from 'os';
import { __testing, type PendingHostWorkEntry } from '../src/commands/migrations/v0_11_0.ts';
@@ -55,8 +55,6 @@ function writeCronJson(dir: string, jobs: unknown[]) {
writeFileSync(path, JSON.stringify({ jobs }, null, 2) + '\n');
return path;
}
// Re-export dirname so writeCronJson can use it without another import
const dirname = (p: string) => p.substring(0, p.lastIndexOf('/'));
const DEFAULT_OPTS = {
yes: true,
@@ -277,13 +275,17 @@ describe('findAgentsMdFiles + findCronManifests scoping', () => {
test('does NOT walk $PWD unless --host-dir is passed', () => {
mkdirSync(join(tmp, 'project'), { recursive: true });
writeFileSync(join(tmp, 'project', 'AGENTS.md'), '# project\n');
// findAgentsMdFiles returns join()-built native paths, so a '/project/'
// literal matches nothing on win32 and the negative assertion below would
// pass vacuously. Only the separator comes from `path`; the directory name
// being probed stays hand-written.
// No --host-dir
const found = findAgentsMdFiles(DEFAULT_OPTS);
expect(found.some(p => p.includes('/project/'))).toBe(false);
expect(found.some(p => p.includes(`${sep}project${sep}`))).toBe(false);
// With --host-dir
const foundWithHostDir = findAgentsMdFiles({ ...DEFAULT_OPTS, hostDir: join(tmp, 'project') });
expect(foundWithHostDir.some(p => p.includes('/project/'))).toBe(true);
expect(foundWithHostDir.some(p => p.includes(`${sep}project${sep}`))).toBe(true);
});
test('findCronManifests picks up cron/jobs.json under scoped roots', () => {
+3 -1
View File
@@ -100,7 +100,9 @@ describe('composeResolvers — mount skills', () => {
expect(result.entries).toHaveLength(1);
expect(result.entries[0].qualifiedName).toBe('yc-media::ingest');
expect(result.entries[0].brainId).toBe('yc-media');
expect(result.entries[0].absolutePath).toContain('/skills/ingest/SKILL.md');
// absolutePath is join()-built, so it carries '\' on win32 — build the
// expected tail with join() rather than a POSIX literal.
expect(result.entries[0].absolutePath).toContain(join('/skills', 'ingest', 'SKILL.md'));
});
test('disabled mount is excluded', () => {
+4 -2
View File
@@ -1,6 +1,6 @@
import { describe, test, expect, afterEach, beforeEach } from 'bun:test';
import { mkdtempSync, rmSync, mkdirSync, writeFileSync, existsSync } from 'fs';
import { join } from 'path';
import { join, isAbsolute } from 'path';
import { tmpdir, homedir } from 'os';
import { __testing } from '../src/commands/mounts.ts';
@@ -52,7 +52,9 @@ describe('parseAddArgs', () => {
expect(parsed.id).toBe('yc-media');
expect(parsed.engine).toBe('pglite');
expect(parsed.database_path).toBe('/tmp/yc-media/.pg');
expect(parsed.path.startsWith('/')).toBe(true);
// parsed.path is resolve()-built: 'C:\…' on win32, so a leading-'/'
// check is the wrong absoluteness test.
expect(isAbsolute(parsed.path)).toBe(true);
});
test('minimal postgres add', () => {
+10 -4
View File
@@ -98,8 +98,10 @@ describe('walkMarkdownFiles', () => {
const out = walkMarkdownFiles(root);
const sorted = out.slice().sort();
expect(sorted.length).toBe(2);
expect(sorted[0]).toBe('meetings/one.md');
expect(sorted[1]).toBe('personal/three.md');
// walkMarkdownFiles builds its relative paths with join(), which emits
// '\' on win32 — build the expectations the same way.
expect(sorted[0]).toBe(join('meetings', 'one.md'));
expect(sorted[1]).toBe(join('personal', 'three.md'));
} finally {
rmSync(root, { recursive: true, force: true });
}
@@ -180,8 +182,12 @@ describe('JSONL utilities', () => {
});
test('default paths resolve under ~/.gbrain/eval/', () => {
expect(defaultMiningOutPath()).toContain('.gbrain/eval/notability-mining-candidates.jsonl');
expect(defaultReviewOutPath()).toContain('.gbrain/eval/notability-real.jsonl');
// Both defaults are join()-built, so the substring must carry native
// separators too ('\.gbrain\eval\…' on win32).
expect(defaultMiningOutPath()).toContain(
join('.gbrain', 'eval', 'notability-mining-candidates.jsonl'),
);
expect(defaultReviewOutPath()).toContain(join('.gbrain', 'eval', 'notability-real.jsonl'));
});
});
+2 -2
View File
@@ -1,5 +1,5 @@
import { describe, test, expect } from 'bun:test';
import { join } from 'path';
import { join, sep } from 'path';
import type { OperationContext } from '../src/core/operations.ts';
import {
buildSkillCatalog,
@@ -158,6 +158,6 @@ describe('crossReferenceTools', () => {
describe('resolveSkillMdPath (happy path)', () => {
test('resolves a real skill to its SKILL.md', () => {
const p = resolveSkillMdPath(FIXTURE, 'brain-ops');
expect(p.endsWith('/brain-ops/SKILL.md')).toBe(true);
expect(p.endsWith(sep + join('brain-ops', 'SKILL.md'))).toBe(true);
});
});
+2 -1
View File
@@ -54,7 +54,8 @@ describe('buildBootstrapDisplay', () => {
workspace: '/ws',
});
expect(r.shown).toBe(false);
expect(r.bootstrapPath).toContain('runbooks/bootstrap.md');
// bootstrapPath is join(packRoot, relPath), so it carries '\' on win32.
expect(r.bootstrapPath).toContain(join('runbooks', 'bootstrap.md'));
});
test('returns shown=false when bootstrap.md is empty (whitespace only)', () => {
+43 -2
View File
@@ -41,7 +41,12 @@ describe('walkSourceDir', () => {
const items = walkSourceDir(src, '/some/dst');
expect(items).toHaveLength(2);
expect(items.map(i => i.target).sort()).toEqual(['/some/dst/a.txt', '/some/dst/b.txt']);
// Build expectations with join(), the same way walkSourceDir builds targets,
// so they carry native separators (byte-identical on POSIX).
expect(items.map(i => i.target).sort()).toEqual([
join('/some/dst', 'a.txt'),
join('/some/dst', 'b.txt'),
]);
});
it('walks nested directories recursively, mirroring structure', () => {
@@ -54,7 +59,11 @@ describe('walkSourceDir', () => {
const items = walkSourceDir(src, '/dst');
expect(items).toHaveLength(3);
const targets = items.map(i => i.target).sort();
expect(targets).toEqual(['/dst/sub/deeper/low.txt', '/dst/sub/mid.txt', '/dst/top.txt']);
expect(targets).toEqual([
join('/dst', 'sub', 'deeper', 'low.txt'),
join('/dst', 'sub', 'mid.txt'),
join('/dst', 'top.txt'),
]);
});
it('returns empty array for a non-existent source directory', () => {
@@ -215,6 +224,38 @@ describe('copyArtifacts — canonical-path containment (harvest path)', () => {
const dst = scratch('copy-dst-');
const result = copyArtifacts(walkSourceDir(skillDir, dst), { confineRealpath: skillDir });
expect(result.summary.wroteNew).toBe(1);
expect(readFileSync(join(dst, 'SKILL.md'), 'utf-8')).toBe('safe');
});
// Both sides of the containment check are realpathSync() output, so the
// prefix separator has to be the native one. A hardcoded '/' never matches
// a win32 realpath, which rejected EVERY source as path_traversal and made
// harvest a dead feature there. These two pin the fix from both directions:
// an in-root source is accepted, and the sibling-prefix guard the separator
// exists to provide still holds. Asserting only the first would pass equally
// well with the containment check deleted.
it('containment boundary holds with native separators (foo does not match foobar)', () => {
const harvestRoot = scratch('copy-harvest-');
const skillDir = join(harvestRoot, 'skills', 'foo');
const siblingDir = join(harvestRoot, 'skills', 'foobar');
mkdirSync(skillDir, { recursive: true });
mkdirSync(siblingDir, { recursive: true });
writeFileSync(join(siblingDir, 'SKILL.md'), 'sibling');
const dst = scratch('copy-dst-');
// Sources live in `skills/foobar`, confinement root is `skills/foo`.
// Rejected only because the prefix carries a trailing separator.
const items = walkSourceDir(siblingDir, dst);
try {
copyArtifacts(items, { confineRealpath: skillDir });
throw new Error('expected copyArtifacts to reject the sibling-prefix source');
} catch (err) {
expect(err).toBeInstanceOf(CopyError);
expect((err as CopyError).code).toBe('path_traversal');
}
expect(existsSync(join(dst, 'SKILL.md'))).toBe(false);
});
});
+4 -1
View File
@@ -74,7 +74,10 @@ describe('runInitScaffold — cathedral default', () => {
writeFileSync(join(dir, 'skills/preexist/SKILL.md'), 'user content');
const result = runInitScaffold({ targetDir: dir, name: 'preexist' });
// The user's SKILL.md should be in filesSkippedExisting
expect(result.filesSkippedExisting.some((p) => p.endsWith('skills/preexist/SKILL.md'))).toBe(true);
// filesSkippedExisting holds join()-built paths — '\' on win32.
expect(
result.filesSkippedExisting.some((p) => p.endsWith(join('skills', 'preexist', 'SKILL.md'))),
).toBe(true);
// And contents preserved.
expect(require('fs').readFileSync(join(dir, 'skills/preexist/SKILL.md'), 'utf-8')).toBe('user content');
});
+10 -7
View File
@@ -14,7 +14,7 @@ import {
utimesSync,
writeFileSync,
} from 'fs';
import { dirname, join } from 'path';
import { dirname, join, sep } from 'path';
import { tmpdir } from 'os';
import {
@@ -169,13 +169,16 @@ describe('enumerateBundle (D-CX-10 dependency closure)', () => {
const m = loadBundleManifest(gbrainRoot);
const entries = enumerateBundle({ gbrainRoot, skillSlug: 'alpha', manifest: m });
const targets = entries.map(e => e.relTarget).sort();
expect(targets).toContain('alpha/SKILL.md');
expect(targets).toContain('alpha/scripts/alpha.mjs');
// relTarget carries native separators — build expectations with join()/sep
// so the positives match and the `beta` exclusion below stays meaningful
// instead of passing vacuously on Windows.
expect(targets).toContain(join('alpha', 'SKILL.md'));
expect(targets).toContain(join('alpha', 'scripts', 'alpha.mjs'));
// Shared deps pulled in despite single-skill scope.
expect(targets).toContain('conventions/quality.md');
expect(targets).toContain(join('conventions', 'quality.md'));
expect(targets).toContain('_output-rules.md');
// beta NOT included.
expect(targets.find(t => t.startsWith('beta/'))).toBeUndefined();
expect(targets.find(t => t.startsWith('beta' + sep))).toBeUndefined();
});
it('throws BundleError for unknown skill slug', () => {
const { gbrainRoot } = scratchGbrain();
@@ -189,8 +192,8 @@ describe('enumerateBundle (D-CX-10 dependency closure)', () => {
const m = loadBundleManifest(gbrainRoot);
const entries = enumerateBundle({ gbrainRoot, manifest: m });
const targets = entries.map(e => e.relTarget).sort();
expect(targets.some(t => t.startsWith('alpha/'))).toBe(true);
expect(targets.some(t => t.startsWith('beta/'))).toBe(true);
expect(targets.some(t => t.startsWith('alpha' + sep))).toBe(true);
expect(targets.some(t => t.startsWith('beta' + sep))).toBe(true);
});
});
+2 -1
View File
@@ -274,7 +274,8 @@ describe('sources_add — remote callers ignore path/clone_dir overrides', () =>
})) as any;
// Clone landed at the SAFE default, not /etc/gbrain-pwned.
expect(row.local_path).not.toBe('/etc/gbrain-pwned');
expect(row.local_path).toContain('clones/attack-clone-dir');
// defaultCloneDir → gbrainPath('clones', id) → join, so '\' on win32.
expect(row.local_path).toContain(join('clones', 'attack-clone-dir'));
// /etc/gbrain-pwned was never written.
expect(existsSync('/etc/gbrain-pwned')).toBe(false);
});
+32 -15
View File
@@ -16,13 +16,30 @@
*/
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from 'fs';
import { join } from 'path';
import { basename, join, sep } from 'path';
import { tmpdir } from 'os';
import { collectSyncableFiles } from '../src/commands/import.ts';
import { withEnv } from './helpers/with-env.ts';
/**
* PLATFORM NOTE: collectSyncableFiles returns absolute paths built with
* join(), so on win32 they carry '\' correct, since callers feed them to
* readFileSync. Two POSIX-only idioms silently no-op there: `split('/').pop()`
* returns the WHOLE path instead of a basename, and `replace(tmp, '')` leaves
* a '\'-prefixed tail. Both turn the `startsWith('/.git')` style negative
* assertions below into vacuous passes, so the skip-list regressions this file
* exists to pin would stop being tested at all.
*
* Expectations therefore stay NATIVE: only the separator comes from `path`,
* the directory structure in each one stays hand-written. Byte-identical to
* the old POSIX literals when sep is '/'.
*/
let tmp: string;
/** Path relative to `tmp`, native separators, leading separator preserved. */
const relToTmp = (p: string): string => p.slice(tmp.length);
beforeEach(() => {
tmp = mkdtempSync(join(tmpdir(), 'gbrain-walker-'));
});
@@ -44,7 +61,7 @@ describe('collectSyncableFiles symlink + cycle hardening', () => {
expect(ms).toBeLessThan(1000); // would hang if walker followed the loop
expect(files).toContain(join(tmp, 'notes.md'));
expect(files.every(f => !f.includes('/loop/'))).toBe(true);
expect(files.every(f => !f.includes(`${sep}loop${sep}`))).toBe(true);
});
});
@@ -82,7 +99,7 @@ describe('collectSyncableFiles symlink + cycle hardening', () => {
const files = collectSyncableFiles(tmp, { strategy: 'markdown' });
expect(files).toContain(join(tmp, 'shallow.md'));
expect(files.every(f => !f.includes('/d35/'))).toBe(true); // past depth 32
expect(files.every(f => !f.includes(`${sep}d35${sep}`))).toBe(true); // past depth 32
});
});
@@ -96,9 +113,9 @@ describe('collectSyncableFiles symlink + cycle hardening', () => {
const markdown = collectSyncableFiles(tmp, { strategy: 'markdown' });
const auto = collectSyncableFiles(tmp, { strategy: 'auto' });
expect(code.map(f => f.split('/').pop()).sort()).toEqual(['bar.py', 'foo.ts']);
expect(markdown.map(f => f.split('/').pop())).toEqual(['notes.md']);
expect(auto.map(f => f.split('/').pop()).sort()).toEqual(['bar.py', 'foo.ts', 'notes.md']);
expect(code.map(f => basename(f)).sort()).toEqual(['bar.py', 'foo.ts']);
expect(markdown.map(f => basename(f))).toEqual(['notes.md']);
expect(auto.map(f => basename(f)).sort()).toEqual(['bar.py', 'foo.ts', 'notes.md']);
});
});
@@ -113,12 +130,12 @@ describe('collectSyncableFiles symlink + cycle hardening', () => {
writeFileSync(join(tmp, 'node_modules/foo/index.md'), 'no\n');
const files = collectSyncableFiles(tmp, { strategy: 'markdown' });
const names = files.map(f => f.replace(tmp, ''));
const names = files.map(f => relToTmp(f));
expect(names).toContain('/real.md');
expect(names.every(n => !n.startsWith('/.git'))).toBe(true);
expect(names.every(n => !n.startsWith('/.claude'))).toBe(true);
expect(names.every(n => !n.startsWith('/node_modules'))).toBe(true);
expect(names).toContain(`${sep}real.md`);
expect(names.every(n => !n.startsWith(`${sep}.git`))).toBe(true);
expect(names.every(n => !n.startsWith(`${sep}.claude`))).toBe(true);
expect(names.every(n => !n.startsWith(`${sep}node_modules`))).toBe(true);
});
});
@@ -130,14 +147,14 @@ describe('collectSyncableFiles symlink + cycle hardening', () => {
// Off → markdown only.
await withEnv({ GBRAIN_EMBEDDING_MULTIMODAL: undefined }, () => {
const off = collectSyncableFiles(tmp, { strategy: 'markdown' });
expect(off.map(f => f.split('/').pop()).sort()).toEqual(['r.md']);
expect(off.map(f => basename(f)).sort()).toEqual(['r.md']);
});
// On → markdown + images (preserves v0.27.1 F2 collectMarkdownFiles
// behavior; codex C5 carve-out).
await withEnv({ GBRAIN_EMBEDDING_MULTIMODAL: 'true' }, () => {
const on = collectSyncableFiles(tmp, { strategy: 'markdown' });
expect(on.map(f => f.split('/').pop()).sort()).toEqual(['j.jpg', 'p.png', 'r.md']);
expect(on.map(f => basename(f)).sort()).toEqual(['j.jpg', 'p.png', 'r.md']);
});
});
@@ -156,8 +173,8 @@ describe('collectSyncableFiles symlink + cycle hardening', () => {
expect(first).toEqual(second);
// Sorted: a.md, b.md, sub/c.md (lexicographic on absolute paths).
expect(first.map(f => f.replace(tmp, ''))).toEqual([
'/a.md', '/b.md', '/sub/c.md',
expect(first.map(f => relToTmp(f))).toEqual([
`${sep}a.md`, `${sep}b.md`, `${sep}sub${sep}c.md`,
]);
});
});