diff --git a/CHANGELOG.md b/CHANGELOG.md index b130dfee2..2c454c7d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [Unreleased] + +**`/cso` now starts and finishes on ordinary repositories that hold a nested Git repository, a long patch history, or a Docker socket on macOS 27.** Three user reports (#3255, #3248) each stopped a run on a normal repo. + +### Fixed +- **An untracked nested Git repository or a registered submodule no longer stops `gstack-cso start` with `UNSAFE_PATH`.** Git lists such a directory as one entry (`inner/` for an untracked repo, `sub` for a gitlink). The snapshot now records it as one explicit `excluded: nested Git repository` entry that is never read or copied, and the snapshot coverage lists it under exclusions. The capture pins the boundary's device, inode and mode, so a directory swapped in during capture fails as `SNAPSHOT_RACE`. A symlink to a directory is still refused. Ported from #3256 by Chao Chen (@surlymochan), who keeps authorship of the fix. (#3255, bug 1) +- **A patch history over 1 MiB no longer leaves `history-inputs` not assessed.** The helper reads up to 100 commits of patches, then keeps the newest whole commits that fit the 1 MiB redaction bound. It never cuts inside a commit, so a secret can't be split across the cut and escape redaction. When even the newest patch is too large, it keeps commit metadata only. A truncated `history.txt` starts with a `[gstack-cso history truncated: …]` line, `history-status.json` records `truncated` and the retained commit count, and the history coverage becomes `partial` with that gap. The redaction bound is unchanged. (#3255, bug 2) + +### For contributors +- **`test/cso-ordinary-repos.test.ts`** covers each report end to end through the compiled launcher and `capture()`. Its negative controls show that a symlinked directory is still refused, that a nested repository swapped during capture fails as a race (this check fails if the device and inode comparison is removed), that retained and omitted history never shows a credential canary, and that Docker endpoint admission still refuses non-sockets, dangling and looping links, `..` paths and TCP. The #3248 socket fallback (`realpathSocket`) shipped in 1.91.96.0. The new endpoint-level case fails when admission calls plain `realpathSync`. + ## [1.91.148.0] - 2026-10-19 **Weekly seed PRs now get the credential scan as well as the free suites.** A pull request the recorder opens with the workflow token never starts its own `pull_request` checks. GitHub holds them as "action required". So the recorder dispatches its checks on the bot branch instead. diff --git a/test/cso-ordinary-repos.test.ts b/test/cso-ordinary-repos.test.ts new file mode 100644 index 000000000..f8dea3a2b --- /dev/null +++ b/test/cso-ordinary-repos.test.ts @@ -0,0 +1,296 @@ +// Regressions for /cso on ordinary user repositories: an untracked nested Git +// repository (#3255 bug 1), a patch history over the 1 MiB bound (#3255 bug 2), +// and a Docker socket that realpath refuses to open (#3248). +import { afterAll, afterEach, beforeAll, describe, expect, spyOn, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as net from 'node:net'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { NESTED_REPOSITORY, capture, historyMarker } from '../lib/cso/snapshot'; +import { newRun, readJson } from '../lib/cso/state'; +import { MAX_OUTPUT } from '../lib/cso/contracts'; +import { dockerEndpoint } from '../lib/cso/docker'; +import { TRUSTED_DIRECTORIES } from '../lib/cso/process'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const launcher = path.join(ROOT, 'bin', process.platform === 'win32' ? 'gstack-cso-launcher.exe' : 'gstack-cso-launcher'); +const MIB = 1024 * 1024; +const CREDENTIAL_CANARY = ['ghp_', 'abcdefghijklmnopqrstuvwxyz1234567890'].join(''); +const OLD_CANARY = ['ghp_', 'zyxwvutsrqponmlkjihgfedcba0987654321'].join(''); +const MARKER = '[gstack-cso history truncated:'; +const roots: string[] = []; +const originalHome = process.env.GSTACK_HOME; +const state = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'cso-ordinary-state-'))); +process.env.GSTACK_HOME = state; +afterEach(() => { + for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); +}); +afterAll(() => { + if (originalHome === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = originalHome; + fs.rmSync(state, { recursive: true, force: true }); +}); + +function tmp(): string { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'cso-ordinary-'))); + roots.push(root); + return root; +} +function git(repo: string, ...args: string[]): string { + const result = spawnSync('/usr/bin/git', ['-C', repo, ...args], { encoding: 'utf8', env: { HOME: path.dirname(repo), PATH: '/usr/bin:/bin' }, timeout: 60_000 }); + if (result.status) throw new Error(result.stderr); + return result.stdout; +} +function init(repo: string): string { + fs.mkdirSync(repo, { recursive: true }); + git(repo, 'init', '-q'); + git(repo, 'config', 'user.email', 'fixture@example.test'); + git(repo, 'config', 'user.name', 'Fixture'); + return repo; +} +function commit(repo: string, files: Record, subject: string): void { + for (const [file, body] of Object.entries(files)) fs.writeFileSync(path.join(repo, file), body); + git(repo, 'add', '-A'); + git(repo, 'commit', '-qm', subject); +} +function filler(tag: string, bytes: number): string { + const lines: string[] = []; + for (let i = 0, size = 0; size < bytes; i++) { + const line = `export const ${tag}${i} = ${i};`; + lines.push(line); + size += line.length + 1; + } + return lines.join('\n') + '\n'; +} +function nestedRepository(outer: string, name: string, files: Record): string { + const inner = init(path.join(outer, name)); + commit(inner, files, 'inner'); + return inner; +} +function cso(repo: string, args: string[]) { + return spawnSync(launcher, args, { cwd: repo, encoding: 'utf8', env: { HOME: path.dirname(repo), GSTACK_HOME: state, PATH: '/usr/bin:/bin' }, timeout: 120_000, maxBuffer: 64 * MIB }); +} +function runFiles(dir: string): string[] { + return fs.readdirSync(dir, { recursive: true, withFileTypes: true }).filter((item) => item.isFile()).map((item) => path.join(item.parentPath, item.name)); +} + +describe('CSO start on a repository with an untracked nested repository (#3255 bug 1)', () => { + test('start succeeds, records the boundary as an explicit exclusion, and never reads the nested source', () => { + const repo = init(path.join(tmp(), 'repo')); + commit(repo, { 'README.md': 'outer\n' }, 'outer'); + nestedRepository(repo, 'inner', { 'secret.txt': `TOKEN=${CREDENTIAL_CANARY}\n` }); + const started = cso(repo, ['start', '--repo', repo, '--scope', 'auth', '--offline']); + expect(started.stderr).toBe(''); + expect(started.status).toBe(0); + const run = JSON.parse(started.stdout), + dir = path.join(state, 'security', 'cso', run.repoId, run.runId), + report = readJson(path.join(dir, 'report.json')), + snapshot = report.coverage.find((item: any) => item.domain === 'snapshot-inputs'); + expect(snapshot.exclusions).toEqual([`inner: ${NESTED_REPOSITORY}`]); + expect(snapshot.gaps).toEqual([]); + for (const file of runFiles(dir)) expect(fs.readFileSync(file, 'utf8')).not.toContain(CREDENTIAL_CANARY); + expect(fs.existsSync(path.join(dir, 'snapshot', 'inner'))).toBe(false); + expect(fs.existsSync(path.join(dir, 'readable', 'inner'))).toBe(false); + }); + + test('negative control: an untracked symlink to a directory is still refused, not treated as a boundary', () => { + const root = tmp(), + repo = init(path.join(root, 'repo')), + outside = path.join(root, 'outside'); + commit(repo, { 'README.md': 'outer\n' }, 'outer'); + nestedRepository(repo, 'inner', { 'f.txt': 'x\n' }); + fs.mkdirSync(outside); + fs.writeFileSync(path.join(outside, 'host.txt'), 'host\n'); + fs.symlinkSync(outside, path.join(repo, 'escape')); + const started = cso(repo, ['start', '--repo', repo, '--scope', 'auth', '--offline']); + expect(started.status).not.toBe(0); + expect(JSON.parse(started.stderr).error).toMatchObject({ code: 'UNSAFE_PATH', message: expect.stringContaining('escape') }); + }); + + test('negative control: a nested repository swapped for another directory during capture fails as a snapshot race', async () => { + const root = tmp(), + repo = init(path.join(root, 'repo')), + target = path.join(repo, 'vendor', 'inner'), + replacement = path.join(root, 'replacement'); + commit(repo, { 'README.md': 'outer\n' }, 'outer'); + fs.mkdirSync(path.join(repo, 'vendor')); + nestedRepository(repo, 'vendor/inner', { 'f.txt': 'x\n' }); + nestedRepository(root, 'replacement', { 'f.txt': 'y\n' }); + const original = fs.lstatSync; + let hits = 0; + const counting = spyOn(fs, 'lstatSync').mockImplementation(((candidate: any, options?: any) => { + if (path.resolve(String(candidate)) === target) hits++; + return options === undefined ? original(candidate) : original(candidate, options); + }) as typeof fs.lstatSync); + try { + await capture(repo, newRun(repo).dir, 'HEAD'); + } finally { + counting.mockRestore(); + } + const last = hits; + expect(last).toBeGreaterThan(1); + hits = 0; + let swapped = false; + const swapping = spyOn(fs, 'lstatSync').mockImplementation(((candidate: any, options?: any) => { + if (path.resolve(String(candidate)) === target && ++hits === last) { + fs.renameSync(target, `${target}.parked`); + fs.renameSync(replacement, target); + swapped = true; + } + return options === undefined ? original(candidate) : original(candidate, options); + }) as typeof fs.lstatSync); + try { + await expect(capture(repo, newRun(repo).dir, 'HEAD')).rejects.toMatchObject({ code: 'SNAPSHOT_RACE', message: 'Source changed before snapshot persistence: vendor/inner' }); + } finally { + swapping.mockRestore(); + } + expect(swapped).toBe(true); + }); +}); + +describe('CSO history capture over the 1 MiB bound (#3255 bug 2)', () => { + test('keeps the newest whole commits that fit, marks the truncation, and still redacts', async () => { + const repo = init(path.join(tmp(), 'repo')); + commit(repo, { 'old.ts': filler('old', 450 * 1024) + `// ${OLD_CANARY}\n` }, 'oldest'); + commit(repo, { 'mid.ts': filler('mid', 450 * 1024) }, 'middle'); + commit(repo, { 'new.ts': filler('new', 300 * 1024) + `// ${CREDENTIAL_CANARY}\n` }, 'newest'); + const run = newRun(repo); + await capture(repo, run.dir); + const status = readJson(path.join(run.dir, 'history-status.json')), + history = fs.readFileSync(path.join(run.dir, 'history.txt'), 'utf8'); + expect(status).toMatchObject({ status: 'captured', commits: 2 }); + expect(status.truncated).toBe(`the newest 2 of 3 commits were retained with patches; 1 older commits were omitted to stay within the ${MAX_OUTPUT}-byte history limit`); + expect(history.startsWith(historyMarker(status.truncated))).toBe(true); + expect(Buffer.byteLength(history)).toBeLessThanOrEqual(MAX_OUTPUT); + expect(status.bytes).toBe(Buffer.byteLength(history)); + expect(history).toContain('Subject: newest'); + expect(history).toContain('Subject: middle'); + expect(history).not.toContain('Subject: oldest'); + expect(history).toContain('diff --git a/new.ts b/new.ts'); + expect(history).not.toContain(CREDENTIAL_CANARY); + expect(history).not.toContain(OLD_CANARY); + }); + + test('falls back to commit metadata when the newest patch alone exceeds the bound, and still redacts', async () => { + const repo = init(path.join(tmp(), 'repo')); + commit(repo, { 'a.ts': 'export const a = 1;\n' }, 'first'); + commit(repo, { 'big.ts': filler('big', 1.2 * MIB) }, `rotate ${CREDENTIAL_CANARY}`); + const run = newRun(repo); + await capture(repo, run.dir); + const status = readJson(path.join(run.dir, 'history-status.json')), + history = fs.readFileSync(path.join(run.dir, 'history.txt'), 'utf8'); + expect(status).toMatchObject({ status: 'captured', commits: 'metadata for at most 100' }); + expect(status.truncated).toMatch(/^patches were withheld because/); + expect(history.startsWith(historyMarker(status.truncated))).toBe(true); + expect(history).toContain('Subject: first'); + expect(history).toContain('Subject: rotate'); + expect(history).not.toContain('diff --git'); + expect(history).not.toContain(CREDENTIAL_CANARY); + }); + + test('a history within the bound is retained whole with no marker', async () => { + const repo = init(path.join(tmp(), 'repo')); + commit(repo, { 'a.ts': 'export const a = 1;\n' }, 'first'); + commit(repo, { 'b.ts': `// ${CREDENTIAL_CANARY}\n` }, 'second'); + const run = newRun(repo); + await capture(repo, run.dir); + const status = readJson(path.join(run.dir, 'history-status.json')), + history = fs.readFileSync(path.join(run.dir, 'history.txt'), 'utf8'); + expect(status).toEqual({ status: 'captured', range: status.range, commits: 'at most 100', bytes: Buffer.byteLength(history) }); + expect(history.startsWith('commit ')).toBe(true); + expect(history).toContain('Subject: first'); + expect(history).not.toContain(MARKER); + expect(history).not.toContain(CREDENTIAL_CANARY); + }); + + test('start reports the truncated history as partial coverage and history stays readable', () => { + const repo = init(path.join(tmp(), 'repo')); + commit(repo, { 'old.ts': filler('old', 700 * 1024) }, 'oldest'); + commit(repo, { 'new.ts': filler('new', 700 * 1024) }, 'newest'); + const started = cso(repo, ['start', '--repo', repo, '--scope', 'auth', '--offline']); + expect(started.stderr).toBe(''); + expect(started.status).toBe(0); + const run = JSON.parse(started.stdout), + report = readJson(path.join(state, 'security', 'cso', run.repoId, run.runId, 'report.json')), + coverage = report.coverage.find((item: any) => item.domain === 'history-inputs'); + expect(coverage.status).toBe('partial'); + expect(coverage.gaps).toEqual([`History truncated: the newest 1 of 2 commits were retained with patches; 1 older commits were omitted to stay within the ${MAX_OUTPUT}-byte history limit`]); + expect(coverage.evidence[0]).toStartWith('Captured 1 from '); + const whole = cso(repo, ['history', run.runId]); + expect(whole.status).toBe(0); + expect(whole.stdout.startsWith(MARKER)).toBe(true); + const scoped = cso(repo, ['history', run.runId, 'new.ts']); + expect(scoped.status).toBe(0); + expect(scoped.stdout.startsWith('commit ')).toBe(true); + expect(scoped.stdout).toContain('diff --git a/new.ts b/new.ts'); + }); +}); + +const dockerInstalled = TRUSTED_DIRECTORIES.some((directory) => fs.existsSync(path.join(directory, 'docker'))); +describe.skipIf(process.platform === 'win32' || !dockerInstalled)('CSO Docker endpoint admission when realpath refuses sockets (#3248)', () => { + let root = ''; + let socketPath = ''; + let server: net.Server; + const realpath = fs.realpathSync; + const refusing = ((candidate: fs.PathLike, ...rest: any[]) => { + let socket = false; + try { + socket = fs.statSync(candidate).isSocket(); + } catch {} + if (socket) { + const error = new Error('EOPNOTSUPP: operation not supported on socket') as NodeJS.ErrnoException; + error.code = 'EOPNOTSUPP'; + throw error; + } + return realpath(candidate, ...rest); + }) as typeof fs.realpathSync; + beforeAll(async () => { + root = realpath(fs.mkdtempSync(path.join(os.tmpdir(), 'cso-endpoint-'))); + socketPath = path.join(root, 'docker.sock'); + server = net.createServer(); + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(socketPath, () => resolve()); + }); + }); + afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + fs.rmSync(root, { recursive: true, force: true }); + }); + afterEach(() => { + (fs.realpathSync as any).mockRestore?.(); + }); + const admit = (host: string) => dockerEndpoint(root, { HOME: root, DOCKER_HOST: host }); + + test('admits a socket reached through a symlinked directory and a symlinked leaf, pinned to its identity', async () => { + const linkedDirectory = path.join(root, 'run'), + leaf = path.join(root, 'colima.sock'); + fs.symlinkSync(root, linkedDirectory); + fs.symlinkSync('docker.sock', leaf); + const stub = spyOn(fs, 'realpathSync').mockImplementation(refusing); + const identity = fs.statSync(socketPath); + for (const host of [`unix://${socketPath}`, `unix://${path.join(linkedDirectory, 'colima.sock')}`]) { + const endpoint = await admit(host); + expect(endpoint).toMatchObject({ socket: socketPath, uri: `unix://${socketPath}`, device: identity.dev, inode: identity.ino }); + } + expect(stub.mock.calls.some(([candidate]) => String(candidate) === socketPath)).toBe(true); + }); + + test('negative controls: non-sockets, dangling or looping links, and non-local URIs are still refused', async () => { + spyOn(fs, 'realpathSync').mockImplementation(refusing); + const file = path.join(root, 'plain.sock'), + dangling = path.join(root, 'dangling.sock'), + loopA = path.join(root, 'loop-a.sock'), + loopB = path.join(root, 'loop-b.sock'); + fs.writeFileSync(file, ''); + fs.symlinkSync(path.join(root, 'missing.sock'), dangling); + fs.symlinkSync(loopB, loopA); + fs.symlinkSync(loopA, loopB); + await expect(admit(`unix://${file}`)).rejects.toMatchObject({ code: 'ISOLATION_FAILED', message: 'Docker endpoint is not a local Unix socket' }); + for (const candidate of [dangling, loopA]) + await expect(admit(`unix://${candidate}`)).rejects.toMatchObject({ code: 'ISOLATION_FAILED', message: 'Pinned local Docker socket is unavailable' }); + await expect(admit(`unix://${root}/../${path.basename(root)}/docker.sock`)).rejects.toMatchObject({ code: 'ISOLATION_FAILED' }); + await expect(admit('tcp://127.0.0.1:2375')).rejects.toMatchObject({ code: 'ISOLATION_FAILED' }); + }); +}); diff --git a/test/cso-snapshot-state.test.ts b/test/cso-snapshot-state.test.ts index 4d47568c6..417d3d921 100644 --- a/test/cso-snapshot-state.test.ts +++ b/test/cso-snapshot-state.test.ts @@ -19,7 +19,7 @@ const unlink=fs.unlinkSync.bind(fs);spyOn(fsModule,'unlinkSync').mockImplementat const {withLock}=await import(${JSON.stringify(path.resolve(import.meta.dir,'../lib/cso/state.ts'))});let outcome='success';try{withLock(dir,()=>{const active=path.join(barrier,'work-active');try{write(active,id,{flag:'wx'});}catch{fs.appendFileSync(path.join(barrier,'overlap'),'overlap\\n');}mark('entered-'+id);if(process.env.RACE_HOLD==='1')waitFor('release-'+id);try{if(fs.readFileSync(active,'utf8')===id)fs.unlinkSync(active);}catch{}});}catch(error){outcome=String(error?.code??error);}write(path.join(barrier,'result-'+id),outcome);`;} describe('CSO dirty snapshot boundary',()=>{ test('captures tracked modifications and nonignored untracked source without changing Git state',async()=>{const p=repo();fs.writeFileSync(path.join(p,'tracked.txt'),'dirty\n');fs.writeFileSync(path.join(p,'new.txt'),'untracked\n');fs.writeFileSync(path.join(p,'.gitignore'),'ignored.txt\n');fs.writeFileSync(path.join(p,'ignored.txt'),'ignored\n');const before=git(p,'status','--porcelain=v1','-z'),run=newRun(p),m=await capture(p,run.dir,'HEAD');expect(fs.readFileSync(path.join(run.dir,'snapshot','tracked.txt'),'utf8')).toBe('dirty\n');expect(fs.readFileSync(path.join(run.dir,'snapshot','new.txt'),'utf8')).toBe('untracked\n');expect(fs.existsSync(path.join(run.dir,'snapshot','ignored.txt'))).toBe(false);expect(git(p,'status','--porcelain=v1','-z')).toBe(before);expect(m.changedPaths).toContain('new.txt');}); - test('records an untracked nested repository as one boundary entry instead of failing the run',async()=>{const p=repo(),inner=path.join(p,'inner');fs.mkdirSync(inner);git(inner,'init','-q');git(inner,'config','user.email','fixture@example.test');git(inner,'config','user.name','Fixture');fs.writeFileSync(path.join(inner,'f.txt'),'x\n');git(inner,'add','.');git(inner,'commit','-qm','inner');const run=newRun(p),manifest=await capture(p,run.dir,'HEAD');assertSnapshot(run.dir,manifest);const entry=manifest.entries.find(e=>e.path==='inner');expect(entry?.transformation).toBe(NESTED_REPOSITORY);expect(entry?.originalHash).toBe('not-read');expect(manifest.entries.map(e=>e.path)).toEqual(['inner','tracked.txt']);expect(manifest.changedPaths).toContain('inner');expect(fs.existsSync(path.join(run.dir,'snapshot','inner'))).toBe(false);expect(fs.existsSync(path.join(run.dir,'readable','inner'))).toBe(false);}); + test('records an untracked nested repository as one boundary entry instead of failing the run',async()=>{const p=repo(),inner=path.join(p,'inner');fs.mkdirSync(inner);git(inner,'init','-q');git(inner,'config','user.email','fixture@example.test');git(inner,'config','user.name','Fixture');fs.writeFileSync(path.join(inner,'f.txt'),'x\n');git(inner,'add','.');git(inner,'commit','-qm','inner');const run=newRun(p),manifest=await capture(p,run.dir,'HEAD');assertSnapshot(run.dir,manifest);const entry=manifest.entries.find(e=>e.path==='inner');expect(entry?.transformation).toBe(NESTED_REPOSITORY);expect(entry?.originalHash).toBe('not-read');expect(manifest.entries.map(e=>e.path).filter(p=>p==='tracked.txt'||p.startsWith('inner'))).toEqual(['inner','tracked.txt']);expect(manifest.changedPaths).toContain('inner');expect(fs.existsSync(path.join(run.dir,'snapshot','inner'))).toBe(false);expect(fs.existsSync(path.join(run.dir,'readable','inner'))).toBe(false);}); test('records a registered submodule gitlink the same way as an untracked nested repository',async()=>{const p=repo(),src=tmp();git(src,'init','-q');git(src,'config','user.email','fixture@example.test');git(src,'config','user.name','Fixture');fs.writeFileSync(path.join(src,'g.txt'),'g\n');git(src,'add','.');git(src,'commit','-qm','sub');git(p,'-c','protocol.file.allow=always','submodule','add','-q',src,'sub');const run=newRun(p),manifest=await capture(p,run.dir,'HEAD');assertSnapshot(run.dir,manifest);expect(manifest.entries.find(e=>e.path==='sub')?.transformation).toBe(NESTED_REPOSITORY);expect(fs.existsSync(path.join(run.dir,'snapshot','sub','g.txt'))).toBe(false);}); test('admits source membership and time before an unbounded startup walk',async()=>{const p=repo();fs.writeFileSync(path.join(p,'second.txt'),'second\n');const tooMany=newRun(p);await expect(capture(p,tooMany.dir,undefined,undefined,{maxEntries:1})).rejects.toMatchObject({code:'MISSING_INPUT'});expect(fs.existsSync(path.join(tooMany.dir,'snapshot'))).toBe(false);const expired=newRun(p);await expect(capture(p,expired.dir,undefined,undefined,{deadlineMs:Date.now()-1})).rejects.toMatchObject({code:'DEADLINE'});expect(fs.existsSync(path.join(expired.dir,'snapshot'))).toBe(false);}); test('includes chmod-only changes in diff scope',async()=>{const p=repo();fs.chmodSync(path.join(p,'tracked.txt'),0o755);const manifest=await capture(p,newRun(p).dir,'HEAD');expect(manifest.changedPaths).toContain('tracked.txt');});