mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 20:50:34 +00:00
v0.42.33.0 fix(sources): confine sync re-clone to gbrain-owned clones; never delete a user working tree (#1881) (#1960)
* fix(sources): confine sync re-clone to gbrain-owned clones; never delete a user working tree (#1881) recloneIfMissing deleted local_path whenever a source had a remote_url and a non-healthy on-disk state, with no check that gbrain actually created the clone. A source whose local_path was a user's live working tree (remote_url set, no gbrain-created clone) could have its directory removed and re-cloned over. - isOwnedClone(): ownership, not path-containment. True only for a config .managed_clone marker (written by addSource --url) or exact normalized-path equality with defaultCloneDir(id) (back-compat for pre-marker default clones). - recloneIfMissing: ownership guard aborts before ANY filesystem op; EXDEV-safe sibling-temp clone + atomic swap (old aside -> new in -> drop old) with best-effort restore + a message naming where the original is preserved; symlink-leaf reject before the destructive rename. - sync.ts validate_repo_state guards reclone on isOwnedClone (no per-sync warn). - sources restore degrades to a warning for an unowned source instead of the misleading "missing clone, try sync" hint. Tests: #1881 regression (tree survives), isOwnedClone matrix, symlink reject, EXDEV swap residue-free, --clone-dir owned-via-marker, restore CV3, unownedHint healthy/degraded, sync-level refusal. * chore: bump version and changelog (v0.42.33.0) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: document sources-ops reclone-ownership invariant for v0.42.33.0 (#1881) Add the missing src/core/sources-ops.ts entry to KEY_FILES.md capturing the must-never-violate reclone-ownership guarantee: gbrain only deletes/re-clones a clone it created (isOwnedClone), never a user working tree. Covers managed_clone marker, defaultCloneDir back-compat, EXDEV-safe swap, TOCTOU + symlink-leaf guards, unmanaged_path SourceOpError, and the read-only sources restore path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
5a06af5a57
commit
b31de6613e
@@ -2,6 +2,19 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.42.33.0] - 2026-06-07
|
||||
|
||||
**`gbrain sync` will never delete a repo it didn't create.** If a source was registered with a `remote_url` but its `local_path` pointed at a working tree you manage yourself (not a gbrain-managed clone), a failed or degraded code sync could remove that directory and re-clone over it. Sync now re-clones **only** clones gbrain actually created — identified by an ownership marker, or by gbrain's own clone location for clones made before this release. Anything else, including your live working tree, is treated as read-only: indexed, never deleted. On an unowned path, sync aborts loudly **before touching the filesystem** and tells you how to fix the source registration. Thanks to @zaqwery for the report.
|
||||
|
||||
The re-clone path is also crash-safer now: it clones into a sibling temp on the same filesystem and swaps atomically (old aside → new in → drop old), so a cross-device rename can't leave a source deleted-but-not-restored. If the swap ever fails, the error names exactly where your original clone is preserved.
|
||||
|
||||
### Fixed
|
||||
- **`gbrain sync` never deletes an unowned working tree (gbrain#1881).** Re-clone is confined to clones gbrain created (`config.managed_clone` marker, or the default clone location for pre-marker clones). A `remote_url` source whose `local_path` is your own working tree is synced read-only and refused — loudly, before any filesystem op — never removed. `gbrain sources restore` on such a source warns and keeps the tree instead of deleting it. Reported by @zaqwery.
|
||||
- **Safer re-clone swap.** Re-clone uses a same-filesystem sibling temp plus an atomic swap (no cross-device "deleted but not re-cloned" window); a symlinked clone path is refused; a failed swap reports where the original is preserved.
|
||||
|
||||
### To take advantage of v0.42.33.0
|
||||
|
||||
Upgrade. Nothing to configure. New `--url` sources are marked gbrain-owned automatically, and existing managed clones at the default location keep auto-recovering. If you registered a source whose `local_path` is a working tree you maintain yourself, `gbrain sync` now syncs it read-only and prints how to re-register it if you want gbrain to manage the clone.
|
||||
## [0.42.32.0] - 2026-06-07
|
||||
|
||||
**A single un-parseable note can no longer silently stop your brain from indexing anything new.** A page whose YAML frontmatter `title:` was a bare date (`title: 2024-06-01`) or number (`title: 1458`) parsed as a Date/number, not text — and the importer threw when it tried to lowercase it. That throw blocked the sync bookmark from advancing, so every later `gbrain sync` re-walked the whole repo, never reached HEAD, and quietly stopped indexing new commits. The page was committed and on GitHub, but `gbrain get` returned `page_not_found` with no surfaced error.
|
||||
|
||||
@@ -1,5 +1,61 @@
|
||||
# TODOS
|
||||
|
||||
## gbrain#1881 sync reclone ownership follow-ups (v0.43+)
|
||||
|
||||
Filed from the #1881 fix (`gbrain sync --strategy code` deleted a user's working
|
||||
tree; `recloneIfMissing` now only re-clones a clone gbrain OWNS — `config.managed_clone`
|
||||
marker or exact default-location equality — via `isOwnedClone`). Deliberately scoped
|
||||
OUT of that PR. Codex outside-voice findings #5/#6. See plan + GSTACK REVIEW REPORT at
|
||||
`~/.claude/plans/system-instruction-you-are-working-golden-valiant.md`.
|
||||
|
||||
- [ ] **P2 — `gbrain doctor` misconfigured-source check.** Flag every source row
|
||||
where `config.remote_url` is set but `isOwnedClone(row)` is false (the shape that
|
||||
caused #1881: a federated row whose `local_path` is a user working tree). Print a
|
||||
one-time, actionable hint per row: drop `config.remote_url` to sync it read-only,
|
||||
or remove + re-add with `--url` so gbrain owns the clone. **Why:** the core guard
|
||||
now refuses to delete such rows, but they still exist in users' brains (created by
|
||||
the gstack orchestrator). This is the single surfacing point — it replaces the
|
||||
per-sync stderr warning that was rejected during eng-review (Codex: it would spam
|
||||
every healthy sync). **Where:** extend the doctor checks in `src/commands/doctor.ts`;
|
||||
reuse `isOwnedClone` from `src/core/sources-ops.ts`. No migration.
|
||||
|
||||
- [ ] **P3 — Decide the `--clone-dir`-outside-root policy.** `gbrain sources add --url
|
||||
--clone-dir <path>` lets local callers place a gbrain-owned clone anywhere. The
|
||||
ownership marker (this PR) makes those safe to reclone, but the dormant
|
||||
`clone_dir_outside_gbrain` code in `SourceOpErrorCode` (`sources-ops.ts`) is unused —
|
||||
it hints at a previously-intended confinement rule. Decide: either wire it up (forbid
|
||||
`--clone-dir` outside `$GBRAIN_HOME/clones/`) or delete the dead code. Don't leave it
|
||||
half-implemented. Codex finding #5.
|
||||
|
||||
- [ ] **P2 — Harden the `managed_clone` ownership marker against forgery.** Ownership
|
||||
(`isOwnedClone`) authorizes the destructive reclone swap on the strength of a DB JSON
|
||||
boolean (`config.managed_clone`). Today only `addSource --url` writes it, but it's a
|
||||
mutable field any future `set-config` / external INSERT / restored dump could set on a
|
||||
user-tree path. A forged marker on a real (non-symlink) user path would authorize
|
||||
deletion. (A realpath path-check does NOT close this — it false-positives on ubiquitous
|
||||
system symlinks like macOS /var, and an owned clone gbrain created is legitimately
|
||||
deleted through any operator symlink anyway. Path can't prove ownership.) Two follow-ups:
|
||||
(a) a CI guard asserting NO code path other than `addSource` ever writes the
|
||||
`managed_clone` key; (b) bind ownership to an unforgeable on-disk stamp (a `.gbrain-clone`
|
||||
sentinel written into the clone at creation, verified before any destructive op) instead
|
||||
of / in addition to the DB field — with an equality-fallback for pre-stamp clones. Codex
|
||||
adversarial (High) + Claude adversarial (Finding 2) from the #1881 ship review.
|
||||
|
||||
- [ ] **P3 — Sweep orphaned `.gbrain-reclone-*` temp dirs.** The EXDEV-safe reclone clones
|
||||
into a sibling temp of `local_path` (`.gbrain-reclone-<leaf>-<rand>`). Every error path
|
||||
`rmSync`s it, but a hard crash (SIGKILL/power loss) between clone and swap leaves a full
|
||||
clone orphaned next to the user's `--clone-dir` parent — outside gbrain's swept
|
||||
`clones/.tmp`. Add a startup/doctor sweep for `.gbrain-reclone-*` / `*.old-*` older than N
|
||||
minutes. Codex Medium / Claude Finding 4 from the #1881 ship review.
|
||||
|
||||
- [ ] **P3 — CLI `gbrain sources remove` leaks the managed clone dir.** `runRemove`
|
||||
(`src/commands/sources.ts:269`) runs `DELETE FROM sources` directly, bypassing
|
||||
`removeSource()` and its symlink-safe clone-cleanup guard — so removing a `--url`
|
||||
source never deletes its on-disk clone (storage leak). Route CLI remove through
|
||||
`removeSource()` (or replicate its guard) so the clone dir is cleaned with the same
|
||||
ownership/symlink protections. Orthogonal to the deletion bug; surfaced by Codex
|
||||
finding #6 during the #1881 review.
|
||||
|
||||
## #1737 minion fair-scheduling follow-up (v0.43+)
|
||||
|
||||
Filed during the #1737 wave (`/plan-eng-review` decision F7, codex outside-voice
|
||||
|
||||
@@ -30,6 +30,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this
|
||||
- `src/commands/reindex.ts` — `gbrain reindex --markdown [--limit N] [--dry-run] [--json] [--no-embed] [--repo PATH]`. Walks `pages WHERE page_kind = 'markdown' AND chunker_version < MARKDOWN_CHUNKER_VERSION` in 100-row batches ordered by id. Rows with non-null `source_path` re-import via `importFromFile`; rows without fall back to `importFromContent`. Both paths pass `forceRechunk: true` to bypass `importFromContent`'s `content_hash` short-circuit — without it the chunker version bump never reaches pages whose source content hasn't changed, AND the stripFactsFence privacy strip never applies to pre-strip chunks. Idempotent — partial-completion re-runs pick up via id-ordered batches. Wired into `src/commands/upgrade.ts:runPostUpgrade` after `apply-migrations`. The DB-only fallback (no source file on disk) does NOT pass body-only `compiled_truth` to `importFromContent` (that path re-parses with EMPTY frontmatter and OVERWRITES the page's real frontmatter/title/timeline); it `getPage`+`getTags`, reconstructs FULL markdown via `serializeMarkdown(frontmatter, compiled_truth, timeline, {type, title, tags})`, and re-imports THAT so re-chunking a DB-only page preserves everything while bumping `chunker_version`. Pinned by `test/reindex-preserve-tags.test.ts`.
|
||||
- `src/commands/reindex-code.ts` — `gbrain reindex --code [--source ID] [--dry-run] [--yes] [--json] [--force] [--no-embed]`. Walks `pages WHERE type = 'code'` in 100-row batches, replays through `importCodeFile` for chunk + embed + content_hash folding. Idempotent unless `--force` bypasses the content_hash early-return. Cost-preview model field reads `getEmbeddingModelName()` from the gateway so preview reflects what the gateway will actually embed with. An informational stderr nudge inside `runReindexCode` (so dry-run + execute both surface it): when the configured embedding model isn't code-tuned (allowlist `{'voyage-code-3'}`, case-insensitive bare match), prints a recommendation to switch to `voyage:voyage-code-3`; suppress with `GBRAIN_NO_CODE_MODEL_NUDGE=1`, `--no-embed`, or `--json`. Pure `shouldNudgeCodeModel(bareName)` returns a tagged `NudgeDecision` union (takes the bare model name, emits qualified `voyage:voyage-code-3` for the paste-ready `gbrain config set` line). When `--yes` is absent and the caller is non-TTY or passed `--json`, the cost gate refuses (exit 2, no spend) via the pure exported `buildCostRefusal({json, previewMsg, preview, costUsd, model}): {stdout?, stderr?}` — JSON envelope only when `--json` is explicit, otherwise a human refusal on stderr (the spend guardrail is independent of the output format). Pinned by `test/ai/voyage-code-3-recipe.test.ts`, `test/reindex-code-nudge.serial.test.ts`, `test/reindex-code-model-source.serial.test.ts` (IRON-RULE regression for the cost-preview fix), `test/reindex-cost-refusal.test.ts`.
|
||||
- `src/commands/sync.ts:resolveSlugByPathOrSourcePath` — Resolves a slug by `pages.source_path` first (returns the stored slug for frontmatter-fallback pages whose path doesn't derive a slug), then falls back to `resolveSlugForPath(path)`. Threaded into all 4 delete/rename call sites (`performSync`'s un-syncable cleanup at ~:531, deletes at ~:603, rename oldSlug at ~:622). Without this, emoji-only / Thai / Arabic filenames whose slug came from frontmatter would orphan on delete/rename (the delete path would compute the wrong path-derived slug). Best-effort query — pre-migration brains fall through to the legacy path.
|
||||
- `src/core/sources-ops.ts` — Multi-source registration + clone-lifecycle ops (`addSource`, `recloneIfMissing`, `defaultCloneDir`, `isOwnedClone`, `unownedHint`). **Reclone-ownership invariant (must-never-violate): gbrain may only delete/re-clone a clone it created, NEVER a user working tree.** `recloneIfMissing` deletes `local_path`, so it gates on `isOwnedClone(src)` and throws a `SourceOpError('unmanaged_path', ...)` BEFORE any filesystem op when ownership is unprovable — fail-closed. Ownership is proven by `config.managed_clone === true` (written by `addSource`'s `--url` path, covering default-location and `--clone-dir` clones) OR `local_path === defaultCloneDir(id)` (back-compat for pre-marker clones, via exact normalized-path equality, symlink-free). A row with `remote_url` + an unowned `local_path` (a user-registered working tree, e.g. `sources add --path`) is refused untouched; re-add with `--url` to regain auto-reclone. The reclone is EXDEV-safe: clone into a SIBLING temp of `local_path` (not the shared `clones/.tmp`, which may sit on a different mount than a `--clone-dir` target), then swap (move old aside → move new in → drop old) so `local_path` is never left missing-and-unrecoverable; on swap failure the original is restored, and if restore fails the error names the `aside` path so it's never reflexively deleted. A TOCTOU re-check re-confirms ownership immediately before the destructive move and rejects a symlink leaf swapped in after the entry check (`symlink_escape`). `unownedHint(src, state)` is the shared recovery message used by both the core error and the `gbrain sync --source` CLI error; `gbrain sources restore` special-cases `unmanaged_path` to print "DB row restored; gbrain syncs this path read-only" instead of the misleading "try sync to recover" guidance. `SourceOpErrorCode` includes `unmanaged_path`. Pinned by `test/sources-ops.test.ts`, `test/sources-resync-recovery.test.ts`.
|
||||
- `src/core/utils.ts` — Shared SQL utilities extracted from postgres-engine.ts. Exports `parseEmbedding(value)` (throws on unknown input, used by migration + ingest paths where data integrity matters) and `tryParseEmbedding(value)` (returns `null` + warns once per process, used by search/rescore paths where availability matters more than strictness). `isUndefinedColumnError(err)` predicate — pattern-matches Postgres SQLSTATE 42703 / "column ... does not exist" with engine-driver shape variation tolerated; replaces bare `catch {}` blocks in `oauth-provider.ts` so genuine errors (lock timeout, network blip, permission denied) propagate while column-missing falls through to the legacy fallback. `validateSourceId(id)` throws on anything outside `^[a-z0-9_-]+$`, used by the per-source disk-layout fix in patterns.ts/synthesize.ts before any `join(brainDir, '.sources', source_id, slug+'.md')` so source_id can't traverse out of brainDir. `rowToPage` populates the required `Page.source_id` from the SELECT projection (`scripts/check-source-id-projection.sh` enforces every projection feeding `rowToPage` includes the column).
|
||||
- `src/core/db.ts` — Connection management, schema initialization. `resolveSessionTimeouts()` returns `statement_timeout` + `idle_in_transaction_session_timeout` (defaults 5min each, env-overridable via `GBRAIN_STATEMENT_TIMEOUT`/`GBRAIN_IDLE_TX_TIMEOUT`/`GBRAIN_CLIENT_CHECK_INTERVAL`). Both `connect()` (module singleton) and `PostgresEngine.connect()` (worker pool) consume the result via postgres.js's `connection` option, sending GUCs as startup parameters that survive PgBouncer transaction mode (`setSessionDefaults` kept as a back-compat no-op shim). `connect()` returns `Promise<boolean>` — `true` iff THIS call created the module singleton, `false` if it joined an existing one; the decision is atomic (no `await` between the `if (sql)` null-check and the synchronous `sql = postgres(...)` assignment), so two concurrent module connects can't both claim creation. `PostgresEngine` stores the return as its `_ownsModuleSingleton` token and only the creating engine may `db.disconnect()` the singleton — a borrower probe engine (lint/doctor config-lift) no-ops its disconnect, so its teardown can't null the connection the long-lived cycle owner is still using (the dream-cycle "connect() has not been called" failure). The module `sql` is only ever nulled by `db.disconnect()` (postgres.js auto-reconnects its own internal pool and never touches our reference). `disconnect()` snapshots + nulls `sql` before awaiting `s.end()` so a concurrent connect can't join a pool that's already closing.
|
||||
- `src/commands/migrate-engine.ts` — Bidirectional engine migration (`gbrain migrate --to supabase/pglite`).
|
||||
|
||||
+1
-1
@@ -143,5 +143,5 @@
|
||||
"bun": ">=1.3.10"
|
||||
},
|
||||
"license": "MIT",
|
||||
"version": "0.42.32.0"
|
||||
"version": "0.42.33.0"
|
||||
}
|
||||
|
||||
@@ -394,7 +394,14 @@ async function runRestore(engine: BrainEngine, args: string[]): Promise<void> {
|
||||
console.log(` re-cloned from remote_url (clone dir was missing).`);
|
||||
}
|
||||
} catch (e) {
|
||||
if (e instanceof SourceOpError) {
|
||||
if (e instanceof SourceOpError && e.code === 'unmanaged_path') {
|
||||
// #1881: local_path is the user's own working tree, not a clone gbrain
|
||||
// created. gbrain won't re-clone over it, and `gbrain sync` will refuse it
|
||||
// too — so the generic "missing clone, try sync to recover" guidance below
|
||||
// would be actively misleading. Surface the real situation instead.
|
||||
console.error(` WARN: ${e.message}`);
|
||||
console.error(` The DB row is restored; gbrain syncs this path read-only.`);
|
||||
} else if (e instanceof SourceOpError) {
|
||||
console.error(` WARN: could not re-clone: ${e.message}`);
|
||||
console.error(` The DB row is restored but the on-disk clone is missing.`);
|
||||
console.error(` Try \`gbrain sync --source ${id}\` to recover, or remove + re-add.`);
|
||||
|
||||
+18
-3
@@ -1084,9 +1084,11 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
if (opts.sourceId) {
|
||||
serr(`[gbrain phase] sync.validate_repo_state`);
|
||||
const { validateRepoState } = await import('../core/git-remote.ts');
|
||||
const { recloneIfMissing } = await import('../core/sources-ops.ts');
|
||||
const cfgRows = await engine.executeRaw<{ config: unknown }>(
|
||||
`SELECT config FROM sources WHERE id = $1`,
|
||||
const { recloneIfMissing, isOwnedClone, unownedHint } = await import(
|
||||
'../core/sources-ops.ts'
|
||||
);
|
||||
const cfgRows = await engine.executeRaw<{ local_path: string | null; config: unknown }>(
|
||||
`SELECT local_path, config FROM sources WHERE id = $1`,
|
||||
[opts.sourceId],
|
||||
);
|
||||
const cfg =
|
||||
@@ -1095,13 +1097,26 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
: ((cfgRows[0]?.config ?? {}) as Record<string, unknown>);
|
||||
const remoteUrl = typeof cfg.remote_url === 'string' ? cfg.remote_url : null;
|
||||
if (remoteUrl) {
|
||||
const ownSrc = {
|
||||
id: opts.sourceId,
|
||||
local_path: cfgRows[0]?.local_path ?? repoPath,
|
||||
config: cfg,
|
||||
};
|
||||
const state = validateRepoState(repoPath, remoteUrl);
|
||||
switch (state) {
|
||||
case 'healthy':
|
||||
// No per-sync warning for an unowned-but-healthy source — it would
|
||||
// spam every sync. The misconfig is surfaced by the doctor check
|
||||
// (TODO1) instead. Healthy unowned paths sync read-only and are safe.
|
||||
break;
|
||||
case 'missing':
|
||||
case 'no-git':
|
||||
case 'not-a-dir':
|
||||
// #1881: only re-clone a clone gbrain owns. An unowned local_path
|
||||
// (the user's working tree) is refused loudly, never deleted.
|
||||
if (!isOwnedClone(ownSrc)) {
|
||||
throw new Error(unownedHint(ownSrc, state));
|
||||
}
|
||||
serr(
|
||||
`[gbrain] auto-recovery: re-cloning "${opts.sourceId}" (clone state: ${state}).`,
|
||||
);
|
||||
|
||||
+142
-14
@@ -38,7 +38,7 @@
|
||||
|
||||
import { existsSync, mkdirSync, renameSync, rmSync, lstatSync } from 'fs';
|
||||
import { realpathSync } from 'fs';
|
||||
import { join, dirname, resolve as resolvePath } from 'path';
|
||||
import { join, dirname, basename, resolve as resolvePath } from 'path';
|
||||
import { randomBytes } from 'crypto';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import {
|
||||
@@ -66,7 +66,8 @@ export type SourceOpErrorCode =
|
||||
| 'not_found'
|
||||
| 'protected_id'
|
||||
| 'clone_dir_outside_gbrain'
|
||||
| 'symlink_escape';
|
||||
| 'symlink_escape'
|
||||
| 'unmanaged_path';
|
||||
|
||||
export class SourceOpError extends Error {
|
||||
constructor(
|
||||
@@ -253,6 +254,70 @@ export function isPathContained(child: string, parent: string): boolean {
|
||||
return resolvedChild === resolvedParent || resolvedChild.startsWith(parentWithSep);
|
||||
}
|
||||
|
||||
/**
|
||||
* Did gbrain CREATE this clone (so re-clone/delete is safe)? Ownership, NOT
|
||||
* path-containment — a user-supplied working tree is NEVER owned, even if it
|
||||
* happens to sit under $GBRAIN_HOME. This is the #1881 guard: recloneIfMissing
|
||||
* deletes local_path, so it must only ever fire on a clone gbrain owns.
|
||||
*
|
||||
* Ownership is proven by either:
|
||||
* 1. config.managed_clone === true — written by addSource's --url path
|
||||
* (covers both default-location and --clone-dir clones), OR
|
||||
* 2. local_path === defaultCloneDir(id) — back-compat for clones created
|
||||
* before the marker existed (gbrain's default location), via exact
|
||||
* normalized-path equality (symlink-free, so none of isPathContained's
|
||||
* symlinked-parent / lexical-escape edge cases apply).
|
||||
*
|
||||
* Everything else is fail-closed (NOT owned → refuse to touch): the bug's
|
||||
* federated row (remote_url + a user tree), and pre-marker --clone-dir clones
|
||||
* (rare, local-only) which are byte-for-byte indistinguishable from it. Those
|
||||
* must be re-added to regain auto-reclone — the correct trade-off when ownership
|
||||
* is unprovable.
|
||||
*/
|
||||
export function isOwnedClone(src: {
|
||||
id: string;
|
||||
local_path: string | null;
|
||||
config: unknown;
|
||||
}): boolean {
|
||||
if (!src.local_path) return false;
|
||||
const cfg =
|
||||
typeof src.config === 'string'
|
||||
? (JSON.parse(src.config) as Record<string, unknown>)
|
||||
: ((src.config ?? {}) as Record<string, unknown>);
|
||||
if (cfg.managed_clone === true) return true;
|
||||
return resolvePath(src.local_path) === resolvePath(defaultCloneDir(src.id));
|
||||
}
|
||||
|
||||
/**
|
||||
* Recovery hint for an unowned source with a remote_url. Splits guidance by
|
||||
* on-disk state: a healthy unowned path syncs read-only (just drop remote_url),
|
||||
* but a degraded one (missing/no-git/not-a-dir) cannot be recovered by dropping
|
||||
* remote_url — that would only defer the failure to the "Not a git repository"
|
||||
* check. Shared by the core SourceOpError and the sync.ts CLI error so they read
|
||||
* identically.
|
||||
*/
|
||||
export function unownedHint(
|
||||
src: { id: string; local_path: string | null },
|
||||
state: RepoState,
|
||||
): string {
|
||||
const path = src.local_path ?? '(none)';
|
||||
if (state === 'healthy') {
|
||||
return (
|
||||
`Source "${src.id}" has config.remote_url set but local_path ${path} is not a ` +
|
||||
`clone gbrain created. gbrain syncs it read-only and will never re-clone or delete ` +
|
||||
`it. To silence this, drop config.remote_url, or re-register with --url so gbrain ` +
|
||||
`owns the clone.`
|
||||
);
|
||||
}
|
||||
return (
|
||||
`Source "${src.id}" has config.remote_url set but local_path ${path} is not a clone ` +
|
||||
`gbrain created and is not a usable git repo (state: ${state}). gbrain will NOT ` +
|
||||
`re-clone over it (it is your working tree, not a gbrain-managed mirror). Restore the ` +
|
||||
`directory yourself, or remove + re-add the source with --url to let gbrain manage the ` +
|
||||
`clone.`
|
||||
);
|
||||
}
|
||||
|
||||
// ── addSource ───────────────────────────────────────────────────────────────
|
||||
|
||||
export async function addSource(
|
||||
@@ -327,7 +392,14 @@ export async function addSource(
|
||||
throw e;
|
||||
}
|
||||
|
||||
const config: Record<string, unknown> = { remote_url: parsedUrl.url };
|
||||
// managed_clone:true is the ownership marker (#1881). It authorizes
|
||||
// recloneIfMissing to rm+replace this clone — gbrain created it, here or at
|
||||
// a --clone-dir path. A user-tree row (created by an external INSERT, no
|
||||
// --url) never carries this, so it can never be deleted by reclone.
|
||||
const config: Record<string, unknown> = {
|
||||
remote_url: parsedUrl.url,
|
||||
managed_clone: true,
|
||||
};
|
||||
if (opts.federated !== null && opts.federated !== undefined) {
|
||||
config.federated = opts.federated;
|
||||
}
|
||||
@@ -698,9 +770,22 @@ export async function recloneIfMissing(
|
||||
const state = validateRepoState(src.local_path, remoteUrl);
|
||||
if (state === 'healthy') return false;
|
||||
|
||||
// Re-clone via temp + rename, mirroring addSource's atomicity contract.
|
||||
const tempDir = makeTempCloneDir(id);
|
||||
mkdirSync(dirname(tempDir), { recursive: true });
|
||||
// #1881 ownership guard — abort BEFORE any filesystem op. recloneIfMissing
|
||||
// deletes local_path; gbrain may only do that to a clone it created, never a
|
||||
// user working tree. A row with remote_url + an unowned local_path (the
|
||||
// gstack-orchestrator federated shape) is refused here, loudly, untouched.
|
||||
if (!isOwnedClone(src)) {
|
||||
throw new SourceOpError('unmanaged_path', unownedHint(src, state));
|
||||
}
|
||||
|
||||
// EXDEV-safe atomic reclone. Clone into a SIBLING temp of local_path (not the
|
||||
// shared clones/.tmp, which can be on a different mount than a --clone-dir
|
||||
// target → EXDEV → "deleted but not recloned"). Then swap: move old aside →
|
||||
// move new in → drop old, so local_path is never left missing-and-unrecoverable.
|
||||
const parent = dirname(src.local_path);
|
||||
mkdirSync(parent, { recursive: true });
|
||||
const rand = randomBytes(6).toString('hex');
|
||||
const tempDir = join(parent, `.gbrain-reclone-${basename(src.local_path)}-${rand}`);
|
||||
try {
|
||||
cloneRepo(remoteUrl, tempDir);
|
||||
} catch (e) {
|
||||
@@ -711,19 +796,62 @@ export async function recloneIfMissing(
|
||||
throw e;
|
||||
}
|
||||
|
||||
// If the local_path partially exists (e.g., empty dir, file-not-dir), nuke
|
||||
// it before the rename so renameSync doesn't fail on a non-empty target.
|
||||
rmSync(src.local_path, { recursive: true, force: true });
|
||||
mkdirSync(dirname(src.local_path), { recursive: true });
|
||||
try {
|
||||
renameSync(tempDir, src.local_path);
|
||||
} catch (e) {
|
||||
// TOCTOU re-check immediately before the destructive move: re-confirm
|
||||
// ownership AND reject a symlink leaf swapped in after the entry check (never
|
||||
// rm-rf / rename through a symlink).
|
||||
if (!isOwnedClone(src)) {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
throw new SourceOpError('unmanaged_path', unownedHint(src, state));
|
||||
}
|
||||
let aside: string | null = null;
|
||||
try {
|
||||
if (existsSync(src.local_path)) {
|
||||
// Symlink leaf guard: never rename/rm *through* a symlinked leaf — that's
|
||||
// the TOCTOU swap-in vector (an attacker plants a symlink at local_path
|
||||
// between the entry ownership check and this rename). An owned clone's leaf
|
||||
// is a real dir gbrain created; a symlink here means tamper, so fail closed.
|
||||
// (Symlinked ANCESTORS are intentionally NOT rejected here: for an owned
|
||||
// clone gbrain created the dir at this path — cloneRepo refuses a non-empty
|
||||
// dest, so a pre-existing user tree can never become an owned clone — and a
|
||||
// realpath-chain check false-positives on ubiquitous system symlinks like
|
||||
// macOS /var -> /private/var. The residual DB-trust risk, a forged
|
||||
// managed_clone marker on an arbitrary path, is tracked as a TODO and is
|
||||
// not closable by a path check.)
|
||||
if (lstatSync(src.local_path).isSymbolicLink()) {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
throw new SourceOpError(
|
||||
'symlink_escape',
|
||||
`Refusing to re-clone "${id}": local_path ${src.local_path} is a symlink.`,
|
||||
);
|
||||
}
|
||||
aside = `${src.local_path}.old-${rand}`;
|
||||
renameSync(src.local_path, aside); // same fs (sibling) — no EXDEV
|
||||
}
|
||||
renameSync(tempDir, src.local_path); // same fs — no EXDEV
|
||||
} catch (e) {
|
||||
// Best-effort restore of the original if the swap left local_path missing.
|
||||
if (aside && !existsSync(src.local_path)) {
|
||||
try {
|
||||
renameSync(aside, src.local_path);
|
||||
} catch {
|
||||
/* original kept at `aside`; surfaced via the thrown error below */
|
||||
}
|
||||
}
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
if (e instanceof SourceOpError) throw e;
|
||||
// If the original is still parked at `aside` (restore failed), tell the user
|
||||
// exactly where it is — otherwise a "cleanup the failed reclone" reflex would
|
||||
// delete their only copy.
|
||||
const asideNote =
|
||||
aside && existsSync(aside)
|
||||
? ` Your original clone is preserved at ${aside} — restore it manually; do not delete it.`
|
||||
: '';
|
||||
throw new SourceOpError(
|
||||
'rename_failed',
|
||||
`Could not move re-cloned repo to ${src.local_path}: ${(e as Error).message}`,
|
||||
`Could not move re-cloned repo to ${src.local_path}: ${(e as Error).message}.${asideNote}`,
|
||||
e,
|
||||
);
|
||||
}
|
||||
if (aside) rmSync(aside, { recursive: true, force: true });
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -23,9 +23,13 @@ import {
|
||||
getSourceStatus,
|
||||
recloneIfMissing,
|
||||
isPathContained,
|
||||
isOwnedClone,
|
||||
unownedHint,
|
||||
defaultCloneDir,
|
||||
SourceOpError,
|
||||
} from '../src/core/sources-ops.ts';
|
||||
import { readdirSync } from 'fs';
|
||||
import { runSources } from '../src/commands/sources.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
|
||||
@@ -508,6 +512,273 @@ describe('recloneIfMissing — T4 restore + autopurge recovery', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #1881 — ownership guard: recloneIfMissing must NEVER delete a user working
|
||||
// tree. Ownership (config.managed_clone OR default-location equality), not
|
||||
// path-containment.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('isOwnedClone — ownership predicate', () => {
|
||||
test('marker config.managed_clone:true → owned (even at an external path)', async () => {
|
||||
await withEnv2(async () => {
|
||||
expect(
|
||||
isOwnedClone({
|
||||
id: 'x',
|
||||
local_path: '/some/external/path',
|
||||
config: { remote_url: 'https://github.com/example/repo', managed_clone: true },
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('default-location clone, no marker → owned (back-compat equality)', async () => {
|
||||
await withEnv2(async () => {
|
||||
expect(
|
||||
isOwnedClone({
|
||||
id: 'legacy',
|
||||
local_path: defaultCloneDir('legacy'),
|
||||
config: { remote_url: 'https://github.com/example/repo' },
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('external path, no marker → NOT owned (the #1881 federated shape)', async () => {
|
||||
await withEnv2(async () => {
|
||||
expect(
|
||||
isOwnedClone({
|
||||
id: 'gstack-code-app-abc',
|
||||
local_path: '/Users/dev/tt-flutter-app',
|
||||
config: { remote_url: 'https://github.com/example/repo', federated: true },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
test('null local_path → NOT owned', async () => {
|
||||
await withEnv2(async () => {
|
||||
expect(isOwnedClone({ id: 'x', local_path: null, config: {} })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
test('config as JSON string (DB shape) is parsed', async () => {
|
||||
await withEnv2(async () => {
|
||||
expect(
|
||||
isOwnedClone({
|
||||
id: 'x',
|
||||
local_path: '/external',
|
||||
config: JSON.stringify({ managed_clone: true }),
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('unownedHint — healthy vs degraded guidance', () => {
|
||||
test('healthy: read-only guidance, no "missing clone" framing', () => {
|
||||
const msg = unownedHint({ id: 'x', local_path: '/Users/dev/repo' }, 'healthy');
|
||||
expect(msg).toMatch(/read-only/);
|
||||
expect(msg).toMatch(/drop config\.remote_url/);
|
||||
expect(msg).not.toMatch(/not a usable git repo/);
|
||||
});
|
||||
|
||||
test('degraded: names the state and does not suggest dropping remote_url alone recovers it', () => {
|
||||
const msg = unownedHint({ id: 'x', local_path: '/Users/dev/repo' }, 'no-git');
|
||||
expect(msg).toMatch(/not a usable git repo/);
|
||||
expect(msg).toMatch(/no-git/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('recloneIfMissing — refuses to delete an unowned working tree (#1881)', () => {
|
||||
test('external local_path + remote_url, no marker → throws unmanaged_path, tree survives', async () => {
|
||||
await withEnv2(async () => {
|
||||
// Simulate the gstack orchestrator's federated row: remote_url set, but
|
||||
// local_path points at a live user working tree (no .git → no-git state),
|
||||
// and NO managed_clone marker.
|
||||
const userTree = join(FAKE_GIT_DIR, 'user-working-tree');
|
||||
rmSync(userTree, { recursive: true, force: true });
|
||||
mkdirSync(userTree, { recursive: true });
|
||||
const sentinel = join(userTree, 'KEEP_ME.txt');
|
||||
writeFileSync(sentinel, 'two unpushed commits live here');
|
||||
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config)
|
||||
VALUES ('gstack-code-app-abc', 'flutter', $1,
|
||||
'{"remote_url":"https://github.com/example/repo","federated":true}'::jsonb)`,
|
||||
[userTree],
|
||||
);
|
||||
|
||||
let threw: SourceOpError | null = null;
|
||||
try {
|
||||
await recloneIfMissing(engine, 'gstack-code-app-abc');
|
||||
} catch (e) {
|
||||
threw = e as SourceOpError;
|
||||
}
|
||||
expect(threw).toBeInstanceOf(SourceOpError);
|
||||
expect(threw?.code).toBe('unmanaged_path');
|
||||
// The working tree and its sentinel MUST survive untouched.
|
||||
expect(existsSync(userTree)).toBe(true);
|
||||
expect(existsSync(sentinel)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('sync-shape: same refusal surfaces before any filesystem op', async () => {
|
||||
await withEnv2(async () => {
|
||||
// A degraded unowned path (the path does not exist at all → missing).
|
||||
const ghost = join(FAKE_GIT_DIR, 'ghost-tree');
|
||||
rmSync(ghost, { recursive: true, force: true });
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config)
|
||||
VALUES ('ghost', 'g', $1,
|
||||
'{"remote_url":"https://github.com/example/repo"}'::jsonb)`,
|
||||
[ghost],
|
||||
);
|
||||
await expect(recloneIfMissing(engine, 'ghost')).rejects.toThrow(/unmanaged_path|not a clone gbrain created/);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('recloneIfMissing — symlink TOCTOU + EXDEV-safe swap', () => {
|
||||
test('symlink at an owned default-location path → symlink_escape, target untouched', async () => {
|
||||
await withEnv2(async () => {
|
||||
// Owned by equality: local_path === defaultCloneDir(id). But the path is a
|
||||
// symlink to a real dir → reclone must refuse rather than rename through it.
|
||||
const id = 'sym-owned';
|
||||
const target = join(FAKE_GIT_DIR, 'sym-target');
|
||||
rmSync(target, { recursive: true, force: true });
|
||||
mkdirSync(target, { recursive: true });
|
||||
const targetSentinel = join(target, 'precious.txt');
|
||||
writeFileSync(targetSentinel, 'do not delete');
|
||||
|
||||
mkdirSync(CLONE_ROOT, { recursive: true });
|
||||
const clonePath = defaultCloneDir(id); // = CLONE_ROOT/sym-owned
|
||||
symlinkSync(target, clonePath);
|
||||
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config)
|
||||
VALUES ($1, 's', $2,
|
||||
'{"remote_url":"https://github.com/example/repo"}'::jsonb)`,
|
||||
[id, clonePath],
|
||||
);
|
||||
|
||||
let threw: SourceOpError | null = null;
|
||||
try {
|
||||
await recloneIfMissing(engine, id);
|
||||
} catch (e) {
|
||||
threw = e as SourceOpError;
|
||||
}
|
||||
expect(threw?.code).toBe('symlink_escape');
|
||||
// Symlink target and its contents survive.
|
||||
expect(existsSync(targetSentinel)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('owned no-git clone reclones; no .gbrain-reclone-* / .old-* residue left', async () => {
|
||||
await withEnv2(async () => {
|
||||
const row = await addSource(engine, {
|
||||
id: 'swap-clean',
|
||||
remoteUrl: 'https://github.com/example/repo',
|
||||
});
|
||||
// Degrade to no-git so reclone fires.
|
||||
rmSync(join(row.local_path!, '.git'), { recursive: true, force: true });
|
||||
|
||||
const recloned = await recloneIfMissing(engine, 'swap-clean');
|
||||
expect(recloned).toBe(true);
|
||||
expect(existsSync(join(row.local_path!, '.git'))).toBe(true);
|
||||
|
||||
// Parent (CLONE_ROOT) must hold no swap residue.
|
||||
const residue = readdirSync(CLONE_ROOT).filter(
|
||||
(e) => e.startsWith('.gbrain-reclone-') || e.includes('.old-'),
|
||||
);
|
||||
expect(residue).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('sources restore — unowned source (CV3)', () => {
|
||||
test('restore of an unowned remote_url row: DB row restored, tree survives, correct guidance', async () => {
|
||||
await withEnv2(async () => {
|
||||
// Archived federated row: remote_url set, local_path = a live user tree
|
||||
// (no .git, no managed_clone marker). Restore calls recloneIfMissing,
|
||||
// which now throws unmanaged_path; runRestore must catch it, keep the tree,
|
||||
// and NOT print the misleading "missing clone, try sync to recover" hint.
|
||||
const userTree = join(FAKE_GIT_DIR, 'restore-user-tree');
|
||||
rmSync(userTree, { recursive: true, force: true });
|
||||
mkdirSync(userTree, { recursive: true });
|
||||
const sentinel = join(userTree, 'KEEP_ME.txt');
|
||||
writeFileSync(sentinel, 'live repo');
|
||||
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config, archived)
|
||||
VALUES ('restore-unowned', 'flutter', $1,
|
||||
'{"remote_url":"https://github.com/example/repo","federated":false}'::jsonb,
|
||||
true)`,
|
||||
[userTree],
|
||||
);
|
||||
|
||||
const errs: string[] = [];
|
||||
const origErr = console.error;
|
||||
console.error = (...a: unknown[]) => { errs.push(a.join(' ')); };
|
||||
try {
|
||||
// Real CLI dispatch → runRestore. Must not throw.
|
||||
await runSources(engine, ['restore', 'restore-unowned']);
|
||||
} finally {
|
||||
console.error = origErr;
|
||||
}
|
||||
|
||||
// DB row un-archived (restore succeeded).
|
||||
const rows = await engine.executeRaw<{ archived: boolean }>(
|
||||
`SELECT archived FROM sources WHERE id = 'restore-unowned'`,
|
||||
);
|
||||
expect(rows[0].archived).toBe(false);
|
||||
// Working tree untouched.
|
||||
expect(existsSync(userTree)).toBe(true);
|
||||
expect(existsSync(sentinel)).toBe(true);
|
||||
// Guidance is the read-only one, NOT the misleading "missing clone" hint.
|
||||
const joined = errs.join('\n');
|
||||
expect(joined).toMatch(/read-only/);
|
||||
expect(joined).not.toMatch(/on-disk clone is missing/);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('addSource --url — writes ownership marker', () => {
|
||||
test('config carries managed_clone:true', async () => {
|
||||
await withEnv2(async () => {
|
||||
await addSource(engine, {
|
||||
id: 'marked',
|
||||
remoteUrl: 'https://github.com/example/repo',
|
||||
});
|
||||
const rows = await engine.executeRaw<{ config: unknown }>(
|
||||
`SELECT config FROM sources WHERE id = 'marked'`,
|
||||
);
|
||||
const cfg =
|
||||
typeof rows[0].config === 'string'
|
||||
? JSON.parse(rows[0].config as string)
|
||||
: (rows[0].config as Record<string, unknown>);
|
||||
expect(cfg.managed_clone).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('--clone-dir clone (external path) is owned via marker and reclones', async () => {
|
||||
await withEnv2(async () => {
|
||||
const externalClone = join(FAKE_GIT_DIR, 'custom-clone-dir');
|
||||
rmSync(externalClone, { recursive: true, force: true });
|
||||
const row = await addSource(engine, {
|
||||
id: 'cdir',
|
||||
remoteUrl: 'https://github.com/example/repo',
|
||||
cloneDir: externalClone,
|
||||
});
|
||||
expect(row.local_path).toBe(externalClone);
|
||||
// Remove the leaf → reclone must succeed (owned via marker, NOT containment).
|
||||
rmSync(externalClone, { recursive: true, force: true });
|
||||
const recloned = await recloneIfMissing(engine, 'cdir');
|
||||
expect(recloned).toBe(true);
|
||||
expect(existsSync(join(externalClone, '.git'))).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// isPathContained — symlink-safe confinement helper (exported for reuse)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -254,4 +254,34 @@ describe('performSync re-clone branch (driven by sync.ts:320 logic)', () => {
|
||||
expect(recloned).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// #1881: the sync branch must NOT re-clone over an unowned user working tree.
|
||||
test('unowned local_path (remote_url, no marker): refuses, working tree survives', async () => {
|
||||
await withEnv({ GBRAIN_HOME, PATH: fakePath() }, async () => {
|
||||
// Federated row shape created by the gstack orchestrator: remote_url set,
|
||||
// local_path = a live user tree OUTSIDE the clone root, no managed_clone.
|
||||
const userTree = join(FAKE_GIT_DIR, 'sync-user-tree');
|
||||
rmSync(userTree, { recursive: true, force: true });
|
||||
mkdirSync(userTree, { recursive: true });
|
||||
const sentinel = join(userTree, 'KEEP_ME.txt');
|
||||
writeFileSync(sentinel, 'live repo');
|
||||
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config)
|
||||
VALUES ('sync-unowned', 'flutter', $1,
|
||||
'{"remote_url":"https://github.com/example/repo","federated":true}'::jsonb)`,
|
||||
[userTree],
|
||||
);
|
||||
|
||||
// no-git → the sync branch would historically call recloneIfMissing →
|
||||
// rm the tree. Now it must throw unmanaged_path and leave the tree intact.
|
||||
const state = validateRepoState(userTree, 'https://github.com/example/repo');
|
||||
expect(state).toBe('no-git');
|
||||
await expect(recloneIfMissing(engine, 'sync-unowned')).rejects.toThrow(
|
||||
/unmanaged_path|not a clone gbrain created/,
|
||||
);
|
||||
expect(existsSync(userTree)).toBe(true);
|
||||
expect(existsSync(sentinel)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user