mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
fix(repo): untrack node_modules symlink, guard against tracked symlinks (#3463)
Commit faf5cdba tracked `node_modules -> /tmp/fleet/repo/node_modules`.
That path exists only on the sandbox that produced it, so every other
clone materialized a dangling symlink and `bun install` aborted with
`ENOENT: could not open the "node_modules" directory`. That also broke
`gbrain upgrade` on bun-link installs, which shells out to bun install
and then prints a manual fallback that fails identically.
Three changes:
- Untrack the symlink (`git rm --cached node_modules`).
- Drop the trailing slash from the .gitignore node_modules patterns. A
`node_modules/` pattern matches directories only, which is why a
symlink of the same name was never ignored in the first place.
- Add scripts/check-no-tracked-symlinks.sh, wired into `bun run verify`
and `check:all`. The .gitignore fix alone is not sufficient, since
`git add -f` bypasses it; the guard fails on any mode-120000 entry.
The repo has no legitimate tracked symlinks, so it starts with an
empty allowlist.
Covered by test/no-tracked-symlinks-guard.test.ts, which builds a
throwaway repo containing the exact symlink shape and asserts the guard
exits 1 and names the offender.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ddd66e1d25
commit
2a17a4dab5
+5
-2
@@ -1,4 +1,7 @@
|
||||
node_modules/
|
||||
# No trailing slash: a bare `node_modules/` pattern matches directories only,
|
||||
# so a *symlink* named node_modules slips past it and can be committed
|
||||
# (that's how the /tmp-pointing symlink in faf5cdba got in). Match any type.
|
||||
node_modules
|
||||
bin/
|
||||
.DS_Store
|
||||
*.log
|
||||
@@ -15,7 +18,7 @@ supabase/.temp/
|
||||
# self-contained binaries (the bun --compile path embeds it via
|
||||
# `import path from 'admin/dist/index.html' with { type: 'file' }`).
|
||||
# Build via: cd admin && bun install && bun run build.
|
||||
admin/node_modules/
|
||||
admin/node_modules
|
||||
.idea
|
||||
eval/reports/
|
||||
eval/data/world-v1/world.html
|
||||
|
||||
@@ -1 +0,0 @@
|
||||
/tmp/fleet/repo/node_modules
|
||||
+2
-1
@@ -48,7 +48,7 @@
|
||||
"check:system-of-record": "scripts/check-system-of-record.sh",
|
||||
"check:admin-scope-drift": "scripts/check-admin-scope-drift.sh",
|
||||
"check:cli-exec": "scripts/check-cli-executable.sh",
|
||||
"check:all": "scripts/check-privacy.sh && scripts/check-proposal-pii.sh && scripts/check-test-real-names.sh && scripts/check-jsonb-pattern.sh && scripts/check-source-id-projection.sh && scripts/check-source-config-leak.sh && scripts/check-progress-to-stdout.sh && scripts/check-no-legacy-getconnection.sh && scripts/check-test-isolation.sh && scripts/check-trailing-newline.sh && scripts/check-wasm-embedded.sh && scripts/check-exports-count.sh && scripts/check-admin-build.sh && scripts/check-admin-scope-drift.sh && scripts/check-cli-executable.sh && scripts/check-skill-brain-first.sh && scripts/check-operations-filter-bypass.sh && scripts/check-gateway-routed-no-direct-anthropic.sh && scripts/check-worker-pool-atomicity.sh && scripts/check-key-files-current-state.sh && scripts/check-no-double-retry.sh && scripts/check-batch-audit-site.sh",
|
||||
"check:all": "scripts/check-privacy.sh && scripts/check-proposal-pii.sh && scripts/check-test-real-names.sh && scripts/check-jsonb-pattern.sh && scripts/check-source-id-projection.sh && scripts/check-source-config-leak.sh && scripts/check-progress-to-stdout.sh && scripts/check-no-tracked-symlinks.sh && scripts/check-no-legacy-getconnection.sh && scripts/check-test-isolation.sh && scripts/check-trailing-newline.sh && scripts/check-wasm-embedded.sh && scripts/check-exports-count.sh && scripts/check-admin-build.sh && scripts/check-admin-scope-drift.sh && scripts/check-cli-executable.sh && scripts/check-skill-brain-first.sh && scripts/check-operations-filter-bypass.sh && scripts/check-gateway-routed-no-direct-anthropic.sh && scripts/check-worker-pool-atomicity.sh && scripts/check-key-files-current-state.sh && scripts/check-no-double-retry.sh && scripts/check-batch-audit-site.sh",
|
||||
"check:gateway-routed": "scripts/check-gateway-routed-no-direct-anthropic.sh",
|
||||
"check:worker-pool-atomicity": "scripts/check-worker-pool-atomicity.sh",
|
||||
"check:doc-history": "scripts/check-key-files-current-state.sh",
|
||||
@@ -76,6 +76,7 @@
|
||||
"check:eval-glossary": "scripts/check-eval-glossary-fresh.sh",
|
||||
"check:test-names": "scripts/check-test-real-names.sh",
|
||||
"check:progress": "scripts/check-progress-to-stdout.sh",
|
||||
"check:no-tracked-symlinks": "scripts/check-no-tracked-symlinks.sh",
|
||||
"check:exports-count": "scripts/check-exports-count.sh",
|
||||
"check:admin-build": "scripts/check-admin-build.sh",
|
||||
"check:admin-embedded": "scripts/check-admin-embedded.sh",
|
||||
|
||||
Executable
+67
@@ -0,0 +1,67 @@
|
||||
#!/usr/bin/env bash
|
||||
# CI guard: fail if any symlink is tracked in git.
|
||||
#
|
||||
# A symlink committed from a build sandbox points at a path that exists on
|
||||
# exactly one machine. Everywhere else the checkout produces a dangling
|
||||
# link, and anything that opens it fails. That is not hypothetical: commit
|
||||
# faf5cdba landed `node_modules -> /tmp/fleet/repo/node_modules`, which made
|
||||
# `bun install` abort with `ENOENT: could not open the "node_modules"
|
||||
# directory` on every fresh clone, and took `gbrain upgrade`'s bun-link path
|
||||
# down with it (the auto-upgrade runs `bun install`, so the printed manual
|
||||
# fallback failed the same way).
|
||||
#
|
||||
# .gitignore alone does not prevent this. A `node_modules/` pattern with a
|
||||
# trailing slash matches directories ONLY, so a symlink of the same name is
|
||||
# never ignored. Dropping the slash closes that hole, but `git add -f` still
|
||||
# walks straight past it. This guard is the backstop.
|
||||
#
|
||||
# The repo has no legitimate tracked symlinks, so the allowlist starts
|
||||
# empty. If you ever need one, add its exact repo-relative path to ALLOWLIST
|
||||
# below and explain why — a relative link that resolves inside the repo is
|
||||
# defensible; an absolute one almost never is.
|
||||
#
|
||||
# Usage: scripts/check-no-tracked-symlinks.sh
|
||||
# Exit: 0 when clean, 1 when a tracked symlink is found.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)"
|
||||
cd "$ROOT"
|
||||
|
||||
# Paths permitted to be tracked symlinks. Empty by design.
|
||||
ALLOWLIST=()
|
||||
|
||||
# Git records symlinks with mode 120000. Field 4 of `ls-files -s` is the path
|
||||
# (tab-separated from the stage number), so cut on the tab to keep paths with
|
||||
# spaces intact.
|
||||
found="$(git ls-files -s | awk '$1 == "120000"' | cut -f2- || true)"
|
||||
|
||||
if [ -n "$found" ]; then
|
||||
filtered="$found"
|
||||
for f in "${ALLOWLIST[@]:-}"; do
|
||||
[ -z "$f" ] && continue
|
||||
filtered="$(echo "$filtered" | grep -vxF "$f" || true)"
|
||||
done
|
||||
|
||||
if [ -n "$filtered" ]; then
|
||||
echo "ERROR: symlink(s) tracked in git:"
|
||||
echo
|
||||
while IFS= read -r path; do
|
||||
[ -z "$path" ] && continue
|
||||
target="$(git cat-file blob ":$path" 2>/dev/null || echo '<unreadable>')"
|
||||
echo " $path -> $target"
|
||||
done <<< "$filtered"
|
||||
echo
|
||||
echo "A committed symlink resolves on the machine that created it and"
|
||||
echo "nowhere else. Untrack it:"
|
||||
echo
|
||||
echo " git rm --cached <path>"
|
||||
echo
|
||||
echo "If the path is build output (node_modules, dist, bin), also confirm"
|
||||
echo "it is covered by .gitignore WITHOUT a trailing slash — a trailing"
|
||||
echo "slash matches directories only and lets the symlink through."
|
||||
exit 1
|
||||
fi
|
||||
fi
|
||||
|
||||
echo "check-no-tracked-symlinks: OK (no tracked symlinks)"
|
||||
@@ -42,6 +42,7 @@ CHECKS=(
|
||||
"check:source-id-projection"
|
||||
"check:source-config-leak"
|
||||
"check:progress"
|
||||
"check:no-tracked-symlinks"
|
||||
"check:test-isolation"
|
||||
"check:wasm"
|
||||
"check:admin-build"
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
/**
|
||||
* Regression guard for scripts/check-no-tracked-symlinks.sh.
|
||||
*
|
||||
* Commit faf5cdba tracked `node_modules -> /tmp/fleet/repo/node_modules`.
|
||||
* That path exists on one build sandbox and nowhere else, so every other
|
||||
* clone got a dangling symlink and `bun install` aborted with
|
||||
* `ENOENT: could not open the "node_modules" directory` — which also took
|
||||
* out `gbrain upgrade`'s bun-link path, since it shells out to bun install.
|
||||
*
|
||||
* `.gitignore` did not stop it: a `node_modules/` pattern with a trailing
|
||||
* slash matches directories only, so the symlink was never ignored. The
|
||||
* pattern is fixed, but `git add -f` still bypasses .gitignore entirely,
|
||||
* so the shell guard is the real backstop. These tests pin (1) the guard
|
||||
* detects a tracked symlink, (2) it stays green on this repo, and (3) it
|
||||
* is actually wired into `bun run verify`.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
import { existsSync, statSync, mkdtempSync, rmSync, writeFileSync, symlinkSync } from 'fs';
|
||||
import { resolve, join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { spawnSync } from 'child_process';
|
||||
|
||||
const REPO_ROOT = resolve(import.meta.dir, '..');
|
||||
const GUARD = resolve(REPO_ROOT, 'scripts/check-no-tracked-symlinks.sh');
|
||||
const VERIFY_DISPATCHER = resolve(REPO_ROOT, 'scripts/run-verify-parallel.sh');
|
||||
|
||||
describe('check-no-tracked-symlinks.sh', () => {
|
||||
it('exists and is executable', () => {
|
||||
expect(existsSync(GUARD)).toBe(true);
|
||||
expect((statSync(GUARD).mode & 0o100) !== 0).toBe(true);
|
||||
});
|
||||
|
||||
it('passes on this repo (no tracked symlinks)', () => {
|
||||
const r = spawnSync('bash', [GUARD], { cwd: REPO_ROOT, encoding: 'utf-8' });
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain('OK');
|
||||
});
|
||||
|
||||
it('fails and names the offender when a symlink is tracked', () => {
|
||||
// Build a throwaway repo rather than poisoning this one's index.
|
||||
const dir = mkdtempSync(join(tmpdir(), 'gbrain-symlink-guard-'));
|
||||
try {
|
||||
const git = (...args: string[]) =>
|
||||
spawnSync('git', args, { cwd: dir, encoding: 'utf-8' });
|
||||
|
||||
git('init', '-q');
|
||||
git('config', 'user.email', 'test@example.com');
|
||||
git('config', 'user.name', 'test');
|
||||
|
||||
writeFileSync(join(dir, 'README.md'), '# fixture\n');
|
||||
// Absolute target that does not exist — the exact shape of the bug.
|
||||
symlinkSync('/tmp/does-not-exist/node_modules', join(dir, 'node_modules'));
|
||||
git('add', '-A');
|
||||
|
||||
const r = spawnSync('bash', [GUARD], { cwd: dir, encoding: 'utf-8' });
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stdout).toContain('node_modules -> /tmp/does-not-exist/node_modules');
|
||||
expect(r.stdout).toContain('git rm --cached');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('is wired into the verify dispatcher', () => {
|
||||
const r = spawnSync('bash', [VERIFY_DISPATCHER, '--dry-list'], {
|
||||
cwd: REPO_ROOT,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
expect(new Set(r.stdout.trim().split('\n'))).toContain('check:no-tracked-symlinks');
|
||||
});
|
||||
});
|
||||
|
||||
describe('.gitignore node_modules patterns', () => {
|
||||
it('match symlinks too (no trailing slash)', () => {
|
||||
const lines = require('fs')
|
||||
.readFileSync(resolve(REPO_ROOT, '.gitignore'), 'utf-8')
|
||||
.split('\n')
|
||||
.map((l: string) => l.trim())
|
||||
.filter((l: string) => l && !l.startsWith('#'));
|
||||
|
||||
// A trailing slash restricts the pattern to directories, which is how
|
||||
// the symlink slipped through. Every node_modules rule must be bare.
|
||||
const offenders = lines.filter((l: string) => /node_modules\/$/.test(l));
|
||||
expect(offenders).toEqual([]);
|
||||
expect(lines).toContain('node_modules');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user