diff --git a/bin/gstack-safe-git b/bin/gstack-safe-git new file mode 100755 index 000000000..c18d16046 --- /dev/null +++ b/bin/gstack-safe-git @@ -0,0 +1,149 @@ +#!/usr/bin/env bash +# gstack-safe-git — run one allowlisted, read-only Git query for audits that +# must not execute project-controlled code (/deslop-shared-libs). +# +# Usage: gstack-safe-git [-C ] [args...] +# +# Every invocation runs `git` with this fixed prefix; callers cannot add or +# override it: +# GIT_OPTIONAL_LOCKS=0 GIT_NO_LAZY_FETCH=1 GIT_TERMINAL_PROMPT=0 +# git --no-pager --no-lazy-fetch --no-replace-objects +# -c core.fsmonitor=false -c log.showSignature=false -c diff.submodule=short +# +# Only query shapes that cannot run clean/process filters, textconv or external +# diff drivers, signature verifiers, pagers, transports, or index/ref writes are +# forwarded. log/show/diff always get --no-ext-diff --no-textconv; diff is only +# between two explicit object IDs. Everything else is refused with exit 2 and +# a one-line message naming the allowed forms. Git's own exit status passes +# through unchanged, including 129 when this Git lacks --no-lazy-fetch. +# +# The script sources nothing and executes only `git` from PATH. +set -euo pipefail + +ALLOWED='allowed: rev-parse, symbolic-ref [--short] , branch --show-current, remote [-v | get-url ], config --get|--get-all|--get-regexp , log, show, ls-tree, cat-file, rev-list, merge-base, for-each-ref, show-ref, grep, diff [-- ...], ls-files --cached --others --exclude-standard -z [-- ...]' + +refuse() { + echo "gstack-safe-git: refused: $1; $ALLOWED" >&2 + exit 2 +} + +dir_args=() +if [ "${1:-}" = "-C" ]; then + [ $# -ge 2 ] || refuse "-C needs a directory" + dir_args=(-C "$2") + shift 2 +fi +[ $# -ge 1 ] || refuse "no subcommand" +sub=$1 +shift +case "$sub" in + -*) refuse "global option '$sub' (only a leading -C is accepted; the safety -c settings are fixed)" ;; + rev-parse|symbolic-ref|branch|remote|config|log|show|ls-tree|cat-file|rev-list|merge-base|for-each-ref|show-ref|grep|diff|ls-files) ;; + *) refuse "'$sub' is not an allowlisted read" ;; +esac + +for arg in "$@"; do + [ "$arg" = "--" ] && break + case "$arg" in + --output|--output=*) refuse "'$arg' writes files" ;; + --ext-diff|--textconv|--filters|--path|--path=*) refuse "'$arg' can run configured diff drivers or filters" ;; + --show-signature|*%G*|*'%(signature'*) refuse "'$arg' runs a signature verifier" ;; + --no-index|--recurse-submodules) refuse "'$arg' reads outside the repository's committed objects" ;; + esac +done + +positional_before_dashdash() { + local count=0 arg + for arg in "$@"; do + [ "$arg" = "--" ] && break + case "$arg" in -*) ;; *) count=$((count + 1)) ;; esac + done + echo "$count" +} + +extra=() +case "$sub" in + rev-parse|ls-tree|cat-file|rev-list|merge-base|for-each-ref|show-ref) ;; + log|show) extra=(--no-ext-diff --no-textconv) ;; + grep) + for arg in "$@"; do + [ "$arg" = "--" ] && break + case "$arg" in + -O*|--open-files-in-pager*) refuse "'$arg' launches a pager program" ;; + esac + done + ;; + symbolic-ref) + for arg in "$@"; do + case "$arg" in + -q|--quiet|--short|--no-recurse) ;; + -*) refuse "symbolic-ref '$arg' is not a read" ;; + esac + done + [ "$(positional_before_dashdash "$@")" = 1 ] || refuse "symbolic-ref reads exactly one ref" + ;; + branch) + [ "$*" = "--show-current" ] || refuse "branch is limited to 'branch --show-current'" + ;; + remote) + case "$*" in + ''|-v|--verbose) ;; + *) + [ "${1:-}" = "get-url" ] || refuse "remote is limited to listing and get-url" + shift_count=0 + for arg in "${@:2}"; do + case "$arg" in + --push|--all) ;; + -*) refuse "remote get-url '$arg'" ;; + *) shift_count=$((shift_count + 1)) ;; + esac + done + [ "$shift_count" = 1 ] || refuse "remote get-url takes one remote name" + ;; + esac + ;; + config) + case "${1:-}" in + --get|--get-all|--get-regexp) ;; + *) refuse "config is limited to --get, --get-all and --get-regexp" ;; + esac + [ $# -ge 2 ] && [ $# -le 3 ] || refuse "config reads take a key and an optional value pattern" + for arg in "${@:2}"; do + case "$arg" in -*) refuse "config '$arg'" ;; esac + done + ;; + diff) + ids=0 + for arg in "$@"; do + [ "$arg" = "--" ] && break + case "$arg" in + --cached|--staged|--merge-base|--merge-base=*) refuse "diff '$arg' compares the index or derived revisions" ;; + -*) ;; + *) + [[ "$arg" =~ ^[0-9a-fA-F]{7,64}$ ]] || refuse "diff operand '$arg' is not an explicit object ID (put paths after --)" + ids=$((ids + 1)) + ;; + esac + done + [ "$ids" = 2 ] || refuse "diff needs exactly two explicit committed object IDs, never the worktree or index" + extra=(--no-ext-diff --no-textconv) + ;; + ls-files) + nul=0 + for arg in "$@"; do + [ "$arg" = "--" ] && break + case "$arg" in + -z) nul=1 ;; + --cached|--others|--exclude-standard|--stage) ;; + *) refuse "ls-files '$arg' (the overlay form is 'ls-files --cached --others --exclude-standard -z [-- ...]')" ;; + esac + done + [ "$nul" = 1 ] || refuse "ls-files output must be NUL-delimited with -z" + ;; +esac + +unset GIT_EXTERNAL_DIFF GIT_CONFIG_PARAMETERS GIT_CONFIG_COUNT +export GIT_OPTIONAL_LOCKS=0 GIT_NO_LAZY_FETCH=1 GIT_TERMINAL_PROMPT=0 +exec git --no-pager --no-lazy-fetch --no-replace-objects \ + -c core.fsmonitor=false -c log.showSignature=false -c diff.submodule=short \ + ${dir_args[@]+"${dir_args[@]}"} "$sub" ${extra[@]+"${extra[@]}"} "$@" diff --git a/deslop-shared-libs/SKILL.md b/deslop-shared-libs/SKILL.md index 7177ef891..dd0b7a45a 100644 --- a/deslop-shared-libs/SKILL.md +++ b/deslop-shared-libs/SKILL.md @@ -66,9 +66,15 @@ changed after writing one. A direct HTTP fallback must also return its response on stdout without creating files; do not replace successful authenticated results with an unauthenticated request and then describe the source as inaccessible. -3. Before local object reads, probe no-lazy-fetch support using the safe Git - prefix below and `rev-parse --is-inside-work-tree`. A successful Git version - check alone is insufficient. If unsupported, use pinned-commit GET API source +3. Run every Git command as `~/.claude/skills/gstack/bin/gstack-safe-git `, never bare `git`; + only the exact diagnostic `git --version` may run bare. The helper fixes the + no-lazy-fetch, lock, pager, fsmonitor, signature and replacement-object + protections and refuses reads that could run filters, drivers, hooks or + transports, naming the allowed forms. Never bypass a refusal with raw `git`. + `diff` takes exactly two explicit committed object IDs, then `--` and paths. + First probe with `~/.claude/skills/gstack/bin/gstack-safe-git rev-parse --is-inside-work-tree`; a Git + version check alone is insufficient. If the probe fails (for example + `unknown option: --no-lazy-fetch`), use pinned-commit GET API source and history reads or disclose unavailable local-history coverage. Never retry object reads without the no-lazy-fetch protection, including by decoding loose objects or packfiles directly. After an unsupported probe, do not inspect Git @@ -79,31 +85,10 @@ changed after writing one. unavailable, continue with clearly labeled raw source and unknown tracking status and revision/history coverage. -The exact diagnostic `git --version` may run without the prefix below: it does -not read repository state or execute configured hooks. It never substitutes for -the guarded capability probe. For every other Git invocation disable optional -locks, pager, fsmonitor, signature verification, replacement objects and lazy fetch. -Signature display can execute a -configured project verifier. Replacement refs must not substitute different contents -under a cited commit ID. Keep submodule diffs short rather than reading their trees. -Use this prefix, including for the capability probe: - -```bash -GIT_OPTIONAL_LOCKS=0 GIT_NO_LAZY_FETCH=1 GIT_TERMINAL_PROMPT=0 \ - git --no-pager --no-lazy-fetch --no-replace-objects \ - -c core.fsmonitor=false -c log.showSignature=false -c diff.submodule=short -``` - -Restrict `git diff` to **two explicit committed object IDs**, with -`--no-ext-diff --no-textconv` and `--` before paths. Use the same disabling -flags for patch-producing `log`/`show` commands. Never use worktree/index diffs, -`git status`, temporary indexes, `add`, `hash-object --path`, or other -normalization helpers: these can execute clean/process filters or alter the index. -Do not execute scripts from the audited project, even to inspect it. - -For the uncommitted overlay, enumerate tracked and nonignored untracked paths with -guarded, NUL-delimited `ls-files --cached --others --exclude-standard -z`, then -inspect raw source with the host's read tools or isolated standard-library reads. +Do not execute scripts from the audited project, even to inspect it. For the +uncommitted overlay, enumerate tracked and nonignored untracked paths with +`~/.claude/skills/gstack/bin/gstack-safe-git ls-files --cached --others --exclude-standard -z`, then inspect +raw source with the host's read tools or isolated standard-library reads. For Python reads, use a trusted interpreter with `python3 -I -S`: repository-local modules can shadow standard-library imports and execute code or write bytecode. Do not add project paths to imports, import project modules, or use runtimes that diff --git a/deslop-shared-libs/SKILL.md.tmpl b/deslop-shared-libs/SKILL.md.tmpl index 15c1f3b65..d85a53e9d 100644 --- a/deslop-shared-libs/SKILL.md.tmpl +++ b/deslop-shared-libs/SKILL.md.tmpl @@ -60,9 +60,15 @@ changed after writing one. A direct HTTP fallback must also return its response on stdout without creating files; do not replace successful authenticated results with an unauthenticated request and then describe the source as inaccessible. -3. Before local object reads, probe no-lazy-fetch support using the safe Git - prefix below and `rev-parse --is-inside-work-tree`. A successful Git version - check alone is insufficient. If unsupported, use pinned-commit GET API source +3. Run every Git command as `{{SAFE_GIT}} `, never bare `git`; + only the exact diagnostic `git --version` may run bare. The helper fixes the + no-lazy-fetch, lock, pager, fsmonitor, signature and replacement-object + protections and refuses reads that could run filters, drivers, hooks or + transports, naming the allowed forms. Never bypass a refusal with raw `git`. + `diff` takes exactly two explicit committed object IDs, then `--` and paths. + First probe with `{{SAFE_GIT}} rev-parse --is-inside-work-tree`; a Git + version check alone is insufficient. If the probe fails (for example + `unknown option: --no-lazy-fetch`), use pinned-commit GET API source and history reads or disclose unavailable local-history coverage. Never retry object reads without the no-lazy-fetch protection, including by decoding loose objects or packfiles directly. After an unsupported probe, do not inspect Git @@ -73,31 +79,10 @@ changed after writing one. unavailable, continue with clearly labeled raw source and unknown tracking status and revision/history coverage. -The exact diagnostic `git --version` may run without the prefix below: it does -not read repository state or execute configured hooks. It never substitutes for -the guarded capability probe. For every other Git invocation disable optional -locks, pager, fsmonitor, signature verification, replacement objects and lazy fetch. -Signature display can execute a -configured project verifier. Replacement refs must not substitute different contents -under a cited commit ID. Keep submodule diffs short rather than reading their trees. -Use this prefix, including for the capability probe: - -```bash -GIT_OPTIONAL_LOCKS=0 GIT_NO_LAZY_FETCH=1 GIT_TERMINAL_PROMPT=0 \ - git --no-pager --no-lazy-fetch --no-replace-objects \ - -c core.fsmonitor=false -c log.showSignature=false -c diff.submodule=short -``` - -Restrict `git diff` to **two explicit committed object IDs**, with -`--no-ext-diff --no-textconv` and `--` before paths. Use the same disabling -flags for patch-producing `log`/`show` commands. Never use worktree/index diffs, -`git status`, temporary indexes, `add`, `hash-object --path`, or other -normalization helpers: these can execute clean/process filters or alter the index. -Do not execute scripts from the audited project, even to inspect it. - -For the uncommitted overlay, enumerate tracked and nonignored untracked paths with -guarded, NUL-delimited `ls-files --cached --others --exclude-standard -z`, then -inspect raw source with the host's read tools or isolated standard-library reads. +Do not execute scripts from the audited project, even to inspect it. For the +uncommitted overlay, enumerate tracked and nonignored untracked paths with +`{{SAFE_GIT}} ls-files --cached --others --exclude-standard -z`, then inspect +raw source with the host's read tools or isolated standard-library reads. For Python reads, use a trusted interpreter with `python3 -I -S`: repository-local modules can shadow standard-library imports and execute code or write bytecode. Do not add project paths to imports, import project modules, or use runtimes that diff --git a/scripts/resolvers/index.ts b/scripts/resolvers/index.ts index 938fed706..c9dfa0671 100644 --- a/scripts/resolvers/index.ts +++ b/scripts/resolvers/index.ts @@ -38,7 +38,7 @@ import { generateThirdPartyActions } from './third-party-actions'; import { generateAsideSetup, generateAsideCookbook, generateAsideResearch, generateUntrustedContentWarning, asideExecPrelude } from './aside'; import { generateCommandReference, generateSnapshotFlags, generateBrowseSetup, generateBrowseFallback } from './browse'; import { generateDesignDocDiscovery } from './design-doc-discovery'; -import { generateSharedLibsRubric } from './shared-libs'; +import { generateSharedLibsRubric, generateSafeGitPath } from './shared-libs'; import { generateTestValueBar, generateTestValueMessage } from './test-value'; import { generateQAScope, generateQAExploratory, generateQAFunctional, generateQAResource, generateQAReview, generateQAReviewPreflight, generateQAMethodReads } from './qa'; @@ -63,6 +63,7 @@ export const RESOLVERS: Record = { THIRD_PARTY_ACTIONS: generateThirdPartyActions, DESIGN_DOC_DISCOVERY: generateDesignDocDiscovery, SHARED_LIBS_RUBRIC: generateSharedLibsRubric, + SAFE_GIT: generateSafeGitPath, SHARED_CODE_REUSE: generateSharedCodeReuse, UNTRUSTED_CONTENT_WARNING: generateUntrustedContentWarning, COMMAND_REFERENCE: generateCommandReference, diff --git a/scripts/resolvers/shared-libs.ts b/scripts/resolvers/shared-libs.ts index bbd496217..4529eee15 100644 --- a/scripts/resolvers/shared-libs.ts +++ b/scripts/resolvers/shared-libs.ts @@ -1,4 +1,8 @@ import type { ResolverFn } from './types'; +import { getHostConfig } from '../../hosts/index'; + +/** The installed read-only Git helper. Always the trusted global runtime, never a repo-local root. */ +export const generateSafeGitPath: ResolverFn = (ctx) => `~/${getHostConfig(ctx.host).globalRoot}/bin/gstack-safe-git`; /** Shared criteria only: the caller owns scope, output, and permission to act. */ export const generateSharedLibsRubric: ResolverFn = () => `### Shared-code evaluation rubric diff --git a/test/codex-e2e-shared-libs.test.ts b/test/codex-e2e-shared-libs.test.ts index 53cbd21a2..1ef943110 100644 --- a/test/codex-e2e-shared-libs.test.ts +++ b/test/codex-e2e-shared-libs.test.ts @@ -8,7 +8,7 @@ import { e2eTierEnabled } from './helpers/e2e-gate'; import { EvalCollector } from './helpers/eval-store'; import { detectBaseBranch, E2E_TOUCHFILES, getChangedFiles, GLOBAL_TOUCHFILES, selectTests } from './helpers/touchfiles'; import { - createSharedLibsFixture, installHostileGitConfig, installSourceShims, readRequests, + createSharedLibsFixture, installHostileGitConfig, installSourceShims, isGuardedGitRequest, readRequests, seedOpportunitySources, sharedReadOnlyViolations, SHARED_LIBS_ROOT, snapshotFixture, } from './helpers/shared-libs-eval-fixture'; @@ -54,6 +54,7 @@ describeCodex('Shared-code audit on live Codex (periodic)', () => { result = await runCodexSkill({ skillDir: path.join(SHARED_LIBS_ROOT, '.agents/skills/gstack-deslop-shared-libs'), skillName: 'deslop-shared-libs', + runtimeRoot: SHARED_LIBS_ROOT, // Extract the actual generated Codex workflow, retaining all standalone rules // and its common rubric without importing an unrelated parent preamble. sections: [ @@ -111,9 +112,7 @@ describeCodex('Shared-code audit on live Codex (periodic)', () => { for (const forbidden of ['status', 'fetch', 'ls-remote', 'pull', 'push', 'clone', 'add', 'write-tree', 'hash-object', 'checkout', 'reset']) { expect(request.args).not.toContain(forbidden); } - expect(request.args).toContain('--no-lazy-fetch'); - expect(request.args).toContain('core.fsmonitor=false'); - expect(request.args).toContain('log.showSignature=false'); + expect(isGuardedGitRequest(request), JSON.stringify(request)).toBe(true); } const apiReads = requests.filter(row => (row.tool === 'gh' && row.args[0] === 'api') || row.tool === 'curl'); expect(apiReads.length).toBeGreaterThan(0); diff --git a/test/gstack-safe-git.test.ts b/test/gstack-safe-git.test.ts new file mode 100644 index 000000000..2f0b80995 --- /dev/null +++ b/test/gstack-safe-git.test.ts @@ -0,0 +1,235 @@ +/** + * bin/gstack-safe-git: the only Git entry point /deslop-shared-libs allows. + * A fake `git` first on PATH records the exact argv and environment the real + * script sends, then delegates to the real Git so hostile repository config + * proves which forms can and cannot execute project-controlled programs. + */ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; + +const SCRIPT = path.join(import.meta.dir, '..', 'bin', 'gstack-safe-git'); +const REAL_GIT = Bun.which('git') || 'git'; +const NODE = Bun.which('node') || process.execPath; +const PREFIX = ['--no-pager', '--no-lazy-fetch', '--no-replace-objects', + '-c', 'core.fsmonitor=false', '-c', 'log.showSignature=false', '-c', 'diff.submodule=short']; +const RECORDED_ENV = ['GIT_OPTIONAL_LOCKS', 'GIT_NO_LAZY_FETCH', 'GIT_TERMINAL_PROMPT', + 'GIT_EXTERNAL_DIFF', 'GIT_CONFIG_PARAMETERS', 'GIT_CONFIG_COUNT']; + +let root = '', repo = '', trace = '', marker = '', base = '', head = ''; +let env: Record = {}; + +const git = (...args: string[]) => { + const result = spawnSync(REAL_GIT, args, { cwd: repo, encoding: 'utf8', timeout: 10_000, env }); + if (result.status !== 0) throw new Error(`git ${args.join(' ')}: ${result.stderr}`); + return result.stdout.trim(); +}; +const safeGit = (args: string[], extraEnv: Record = {}, cwd = repo) => + spawnSync(SCRIPT, args, { cwd, encoding: 'utf8', timeout: 10_000, env: { ...env, ...extraEnv } }); +const recorded = (): Array<{ args: string[]; env: Record }> => + fs.existsSync(trace) ? fs.readFileSync(trace, 'utf8').split('\n').filter(Boolean).map(line => JSON.parse(line)) : []; +const hooks = () => fs.existsSync(marker) ? fs.readFileSync(marker, 'utf8') : ''; +const reset = () => { fs.rmSync(trace, { force: true }); fs.rmSync(marker, { force: true }); }; + +beforeAll(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-safe-git-')); + repo = path.join(root, 'repo'); + trace = path.join(root, 'git-calls.jsonl'); + marker = path.join(root, 'hooks.log'); + const bin = path.join(root, 'bin'); + fs.mkdirSync(repo); + fs.mkdirSync(bin); + const gitConfig = path.join(root, 'gitconfig'); + fs.writeFileSync(gitConfig, ''); + env = { ...process.env as Record, GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: gitConfig, + PATH: `${bin}${path.delimiter}${process.env.PATH || ''}` }; + for (const key of RECORDED_ENV) delete env[key]; + // This Git may predate --no-lazy-fetch (2.44); the recorded argv is still exactly what the script sent. + const lazyFlag = spawnSync(REAL_GIT, ['--no-lazy-fetch', '--version'], { encoding: 'utf8', timeout: 10_000 }).status === 0; + fs.writeFileSync(path.join(bin, 'git'), `#!${NODE} +const fs = require('node:fs'), cp = require('node:child_process'); +const args = process.argv.slice(2); +const env = Object.fromEntries(${JSON.stringify(RECORDED_ENV)}.filter(k => k in process.env).map(k => [k, process.env[k]])); +fs.appendFileSync(${JSON.stringify(trace)}, JSON.stringify({ args, env }) + '\\n'); +if (process.env.FAKE_GIT_EXIT) { process.stderr.write('unknown option: --no-lazy-fetch\\n'); process.exit(Number(process.env.FAKE_GIT_EXIT)); } +const forwarded = ${lazyFlag} ? args : args.filter(arg => arg !== '--no-lazy-fetch'); +const result = cp.spawnSync(${JSON.stringify(REAL_GIT)}, forwarded, { stdio: 'inherit', timeout: 10_000 }); +process.exit(result.status ?? 1); +`, { mode: 0o755 }); + + git('init', '-b', 'main'); + git('config', 'user.name', 'Safe Git Fixture'); + git('config', 'user.email', 'safe-git@example.invalid'); + git('remote', 'add', 'origin', 'https://example.invalid/fixture.git'); + fs.writeFileSync(path.join(repo, 'tracked.txt'), 'hello\n'); + fs.writeFileSync(path.join(repo, '.gitattributes'), 'tracked.txt filter=probe diff=probe\n'); + git('add', '.'); + git('commit', '-m', 'base'); + base = git('rev-parse', 'HEAD'); + fs.writeFileSync(path.join(repo, 'tracked.txt'), 'hello world\n'); + git('commit', '-am', 'change'); + // A signature header makes plain log reads consult the configured verifier. + const signed = git('cat-file', 'commit', 'HEAD').replace('\n\n', + '\ngpgsig -----BEGIN PGP SIGNATURE-----\n dummy\n -----END PGP SIGNATURE-----\n\n') + '\n'; + const written = spawnSync(REAL_GIT, ['hash-object', '-t', 'commit', '-w', '--stdin'], + { cwd: repo, input: signed, encoding: 'utf8', timeout: 10_000, env }); + head = written.stdout.trim(); + git('update-ref', 'HEAD', head); + const hook = (name: string, body: string) => { + const file = path.join(root, name); + fs.writeFileSync(file, `#!/bin/sh\necho ${name} >> '${marker}'\n${body}\n`, { mode: 0o755 }); + return file; + }; + git('config', 'filter.probe.clean', hook('clean-hook', 'cat')); + git('config', 'diff.probe.textconv', hook('textconv-hook', 'cat "$1"')); + git('config', 'diff.external', hook('external-diff-hook', 'exit 0')); + git('config', 'core.fsmonitor', hook('fsmonitor-hook', 'exit 1')); + git('config', 'gpg.program', hook('gpg-hook', 'exit 1')); + git('config', 'log.showSignature', 'true'); + // A raw worktree edit: any index refresh or worktree diff would run the clean filter. + fs.writeFileSync(path.join(repo, 'tracked.txt'), 'hello raw overlay\n'); + fs.writeFileSync(path.join(repo, 'untracked.txt'), 'new\n'); +}); + +afterAll(() => { if (root) fs.rmSync(root, { recursive: true, force: true }); }); + +describe('gstack-safe-git', () => { + test('the hostile config is live: equivalent raw Git reads execute every canary', () => { + reset(); + for (const args of [['diff', base, head], ['log', '-1'], ['status'], ['diff', '--no-ext-diff', base, head], + ['hash-object', '--path', 'tracked.txt', 'tracked.txt']]) { + spawnSync(REAL_GIT, args, { cwd: repo, encoding: 'utf8', timeout: 10_000, env }); + } + for (const name of ['external-diff-hook', 'gpg-hook', 'fsmonitor-hook', 'clean-hook', 'textconv-hook']) { + expect(hooks()).toContain(name); + } + }); + + test('always applies the fixed prefix and environment, and strips caller config overrides', () => { + reset(); + const result = safeGit(['rev-parse', '--is-inside-work-tree'], { + GIT_CONFIG_PARAMETERS: "'core.fsmonitor'='/bin/false'", GIT_CONFIG_COUNT: '1', + GIT_EXTERNAL_DIFF: path.join(root, 'external-diff-hook'), + }); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout.trim()).toBe('true'); + expect(recorded()).toEqual([{ args: [...PREFIX, 'rev-parse', '--is-inside-work-tree'], + env: { GIT_OPTIONAL_LOCKS: '0', GIT_NO_LAZY_FETCH: '1', GIT_TERMINAL_PROMPT: '0' } }]); + }); + + const permitted = (): Array<[string[], string[]]> => [ + [['rev-parse', 'HEAD'], ['rev-parse', 'HEAD']], + [['symbolic-ref', '--short', 'HEAD'], ['symbolic-ref', '--short', 'HEAD']], + [['branch', '--show-current'], ['branch', '--show-current']], + [['remote', '-v'], ['remote', '-v']], + [['remote', 'get-url', 'origin'], ['remote', 'get-url', 'origin']], + [['config', '--get', 'remote.origin.url'], ['config', '--get', 'remote.origin.url']], + [['log', '-p', '--format=%H %s', '-2'], ['log', '--no-ext-diff', '--no-textconv', '-p', '--format=%H %s', '-2']], + [['show', head], ['show', '--no-ext-diff', '--no-textconv', head]], + [['show', `${head}:tracked.txt`], ['show', '--no-ext-diff', '--no-textconv', `${head}:tracked.txt`]], + [['ls-tree', '-r', head], ['ls-tree', '-r', head]], + [['cat-file', '-p', head], ['cat-file', '-p', head]], + [['rev-list', '--count', head], ['rev-list', '--count', head]], + [['merge-base', base, head], ['merge-base', base, head]], + [['for-each-ref', '--format=%(refname)'], ['for-each-ref', '--format=%(refname)']], + [['show-ref'], ['show-ref']], + [['grep', '-n', 'hello', head, '--', 'tracked.txt'], ['grep', '-n', 'hello', head, '--', 'tracked.txt']], + [['diff', base, head, '--', 'tracked.txt'], ['diff', '--no-ext-diff', '--no-textconv', base, head, '--', 'tracked.txt']], + [['diff', '--stat', base.slice(0, 12), head], ['diff', '--no-ext-diff', '--no-textconv', '--stat', base.slice(0, 12), head]], + [['ls-files', '--cached', '--others', '--exclude-standard', '-z'], ['ls-files', '--cached', '--others', '--exclude-standard', '-z']], + [['ls-files', '--stage', '-z', '--', 'tracked.txt'], ['ls-files', '--stage', '-z', '--', 'tracked.txt']], + ]; + + test('forwards every permitted read with patch drivers disabled and runs no project program', () => { + for (const [args, forwarded] of permitted()) { + reset(); + const result = safeGit(args); + expect(result.status, `${args.join(' ')}: ${result.stderr}`).toBe(0); + expect(recorded().map(row => row.args), args.join(' ')).toEqual([[...PREFIX, ...forwarded]]); + expect(hooks(), args.join(' ')).toBe(''); + } + reset(); + const overlay = safeGit(['ls-files', '--cached', '--others', '--exclude-standard', '-z']); + expect(overlay.stdout.split('\0').filter(Boolean).sort()).toEqual(['.gitattributes', 'tracked.txt', 'untracked.txt']); + const patch = safeGit(['diff', base, head, '--', 'tracked.txt']); + expect(patch.stdout).toContain('+hello world'); + expect(safeGit(['show', `${head}:tracked.txt`]).stdout).toBe('hello world\n'); + const fromOutside = safeGit(['-C', repo, 'rev-parse', 'HEAD'], {}, root); + expect(fromOutside.stdout.trim()).toBe(head); + expect(hooks()).toBe(''); + }); + + const refused: Array<[string[], RegExp]> = [ + [[], /no subcommand/], + [['status'], /'status' is not an allowlisted read/], + [['add', 'tracked.txt'], /'add' is not an allowlisted read/], + [['hash-object', '--path', 'tracked.txt', 'tracked.txt'], /'hash-object' is not an allowlisted read/], + [['update-index', '--refresh'], /not an allowlisted read/], + [['write-tree'], /not an allowlisted read/], + [['fetch', 'origin'], /not an allowlisted read/], + [['ls-remote', 'origin'], /not an allowlisted read/], + [['checkout', 'main'], /not an allowlisted read/], + [['blame', 'tracked.txt'], /not an allowlisted read/], + [['-c', 'core.fsmonitor=/bin/true', 'log'], /global option '-c'/], + [['--git-dir=.git', 'log'], /global option/], + [['-C'], /-C needs a directory/], + [['diff'], /exactly two explicit committed object IDs/], + [['diff', 'HEAD~1', 'HEAD'], /'HEAD~1' is not an explicit object ID/], + [['diff', '--cached', 'BASE', 'HEAD'], /compares the index/], + [['diff', '--merge-base', 'BASE', 'HEAD'], /compares the index/], + [['diff', 'BASE', 'tracked.txt'], /'tracked.txt' is not an explicit object ID/], + [['diff', 'BASE', '--', 'tracked.txt'], /exactly two explicit committed object IDs/], + [['diff', '--no-index', 'a', 'b'], /'--no-index'/], + [['diff', '--ext-diff', 'BASE', 'HEAD'], /diff drivers or filters/], + [['log', '--output=out.patch', '-p'], /writes files/], + [['log', '-p', '--output', 'out.patch'], /writes files/], + [['log', '--show-signature'], /signature verifier/], + [['log', '--format=%G?'], /signature verifier/], + [['for-each-ref', '--format=%(signature)'], /signature verifier/], + [['show', '--textconv', 'HEAD:tracked.txt'], /diff drivers or filters/], + [['cat-file', '--filters', 'HEAD:tracked.txt'], /diff drivers or filters/], + [['cat-file', '--textconv', 'HEAD:tracked.txt'], /diff drivers or filters/], + [['grep', '-Ocat', 'hello'], /launches a pager program/], + [['grep', '--open-files-in-pager=cat', 'hello'], /launches a pager program/], + [['grep', '--recurse-submodules', 'hello'], /reads outside/], + [['ls-files', '--cached', '--others', '--exclude-standard'], /NUL-delimited with -z/], + [['ls-files', '--modified', '-z'], /ls-files '--modified'/], + [['ls-files', '-z', '--deleted'], /ls-files '--deleted'/], + [['symbolic-ref', 'HEAD', 'refs/heads/other'], /exactly one ref/], + [['symbolic-ref', '-d', 'HEAD'], /is not a read/], + [['branch', 'other'], /branch --show-current/], + [['remote', 'add', 'x', 'https://example.invalid/x.git'], /listing and get-url/], + [['remote', 'show', 'origin'], /listing and get-url/], + [['config', 'user.name', 'x'], /--get, --get-all and --get-regexp/], + [['config', '--get', 'user.name', '--unset'], /config '--unset'/], + ]; + + test.each(refused)('refuses %j with one actionable line and never starts Git', (args, reason) => { + reset(); + const concrete = args.map(arg => arg === 'BASE' ? base : arg === 'HEAD' && args[0] === 'diff' ? head : arg); + const result = safeGit(concrete); + expect(result.status).toBe(2); + expect(result.stdout).toBe(''); + expect(result.stderr.trimEnd().split('\n')).toHaveLength(1); + expect(result.stderr).toMatch(/^gstack-safe-git: refused: /); + expect(result.stderr).toMatch(reason); + expect(result.stderr).toContain('; allowed: rev-parse'); + expect(result.stderr).toContain('diff [-- ...]'); + expect(result.stderr).toContain('ls-files --cached --others --exclude-standard -z'); + expect(recorded()).toEqual([]); + expect(hooks()).toBe(''); + }); + + test("passes Git's exit status and stderr through unchanged", () => { + reset(); + const unsupported = safeGit(['rev-parse', '--is-inside-work-tree'], { FAKE_GIT_EXIT: '129' }); + expect(unsupported.status).toBe(129); + expect(unsupported.stderr).toBe('unknown option: --no-lazy-fetch\n'); + const missing = safeGit(['rev-parse', '--verify', '--quiet', 'refs/heads/absent']); + expect(missing.status).toBe(1); + const badObject = safeGit(['cat-file', '-t', '0'.repeat(40)]); + expect(badObject.status).toBe(128); + }); +}); diff --git a/test/helpers/codex-session-runner.ts b/test/helpers/codex-session-runner.ts index 03a6c6ea6..b659a8fe4 100644 --- a/test/helpers/codex-session-runner.ts +++ b/test/helpers/codex-session-runner.ts @@ -133,6 +133,7 @@ export function installSkillToTempHome( skillName: string, tempHome?: string, sections?: string[], + runtimeRoot?: string, ): string { const home = tempHome || fs.mkdtempSync(path.join(os.tmpdir(), 'codex-e2e-')); const destDir = path.join(home, '.codex', 'skills', skillName); @@ -149,6 +150,11 @@ export function installSkillToTempHome( // nonexistent skill in its response and otherwise pass discovery checks. fs.copyFileSync(srcSkill, path.join(destDir, 'SKILL.md')); } + if (runtimeRoot) { + // The temp HOME has no installed gstack runtime; point runtime helpers at the one under test. + const installed = path.join(destDir, 'SKILL.md'); + fs.writeFileSync(installed, fs.readFileSync(installed, 'utf8').replaceAll('~/.codex/skills/gstack', runtimeRoot)); + } const srcOpenAIYaml = path.join(skillDir, 'agents', 'openai.yaml'); if (fs.existsSync(srcOpenAIYaml)) { @@ -180,6 +186,7 @@ export async function runCodexSkill(opts: { configOverrides?: string[]; // TOML key=value overrides (passed with -c) ignoreUserConfig?: boolean; // Add --ignore-user-config; auth still comes from CODEX_HOME signal?: AbortSignal; // Abort the process group when an enclosing eval expires + runtimeRoot?: string; // gstack runtime that ~/.codex/skills/gstack helper paths resolve to }): Promise { const { skillDir, @@ -193,6 +200,7 @@ export async function runCodexSkill(opts: { configOverrides = [], ignoreUserConfig = false, signal, + runtimeRoot, } = opts; const startTime = Date.now(); @@ -223,7 +231,7 @@ export async function runCodexSkill(opts: { const realHome = os.homedir(); try { - installSkillToTempHome(skillDir, name, tempHome, sections); + installSkillToTempHome(skillDir, name, tempHome, sections, runtimeRoot); // Copy authentication only. Copying the whole operator ~/.codex tree leaks // plugins, MCP servers, rules, memories, and skills into a supposedly diff --git a/test/helpers/shared-libs-eval-fixture.ts b/test/helpers/shared-libs-eval-fixture.ts index a045547d5..284e8a7ea 100644 --- a/test/helpers/shared-libs-eval-fixture.ts +++ b/test/helpers/shared-libs-eval-fixture.ts @@ -250,7 +250,7 @@ export function snapshotFixture(directory: string): Record { export interface SourceRequest { tool: string; args: string[]; endpoint?: string; method?: string; cwd: string; - violation?: string; + env?: Record; violation?: string; pid?: number; ppid?: number; parentExecutable?: string; parentCommand?: string; } @@ -463,6 +463,22 @@ export function sharedReadOnlyViolations(toolCalls: Array<{ tool: string; input: return [...new Set(violations)]; } +const SAFE_GIT_ENV = { GIT_OPTIONAL_LOCKS: '0', GIT_NO_LAZY_FETCH: '1', GIT_TERMINAL_PROMPT: '0' }; +const SAFE_GIT_FLAGS = ['--no-pager', '--no-lazy-fetch', '--no-replace-objects']; +const SAFE_GIT_CONFIG = ['core.fsmonitor=false', 'log.showSignature=false', 'diff.submodule=short']; + +/** A repository Git process that carries the complete safe prefix bin/gstack-safe-git applies, including its + * environment, plus patch-driver disabling for diffs; a partial literal prefix does not qualify. */ +export function isGuardedGitRequest(request: SourceRequest): boolean { + const args = request.args, command = args.findIndex((arg, i) => !arg.startsWith('-') && args[i - 1] !== '-c' && args[i - 1] !== '-C'); + const globals = command < 0 ? args : args.slice(0, command), rest = command < 0 ? [] : args.slice(command); + const configs = globals.flatMap((arg, i) => globals[i - 1] === '-c' ? [arg] : []); + return Object.entries(SAFE_GIT_ENV).every(([key, value]) => request.env?.[key] === value) + && SAFE_GIT_FLAGS.every(flag => globals.includes(flag)) && SAFE_GIT_CONFIG.every(config => configs.includes(config)) + && !rest.some(arg => ['--ext-diff', '--textconv', '--output'].includes(arg) || arg.startsWith('--output=')) + && (rest[0] !== 'diff' || (rest.includes('--no-ext-diff') && rest.includes('--no-textconv'))); +} + /** Claude's own workspace probes are not commands requested by the skill. */ export function isInternalClaudeGitRequest(request: SourceRequest, commands: string[]): boolean { const hostPrefix = ['-c', 'protocol.ext.allow=never', '-c', 'submodule.recurse=false', @@ -578,7 +594,8 @@ export function installSourceShims(f: SharedLibsFixture, opts: { } const common = `const fs=require('node:fs'), cp=require('node:child_process');\nconst a=process.argv.slice(2);\nconst trace=${JSON.stringify(f.trace)};\nconst parent={pid:process.pid,ppid:process.ppid};try{parent.parentExecutable=fs.readlinkSync('/proc/'+process.ppid+'/exe');parent.parentCommand=fs.readFileSync('/proc/'+process.ppid+'/cmdline','utf8').replaceAll('\\0',' ');}catch{try{const info=cp.spawnSync('ps',['-p',String(process.ppid),'-o','comm=','-o','args='],{encoding:'utf8',timeout:3_000});const line=(info.stdout||'').trim();parent.parentExecutable=line.split(/\\s+/)[0];parent.parentCommand=line;}catch{}}\n`; fs.writeFileSync(path.join(f.bin, 'git'), `#!${nodeBin}\n${common} -fs.appendFileSync(trace,JSON.stringify({tool:'git',args:a,cwd:process.cwd(),...parent})+'\\n'); +const gitEnv=Object.fromEntries(['GIT_OPTIONAL_LOCKS','GIT_NO_LAZY_FETCH','GIT_TERMINAL_PROMPT'].filter(k=>k in process.env).map(k=>[k,process.env[k]])); +fs.appendFileSync(trace,JSON.stringify({tool:'git',args:a,env:gitEnv,cwd:process.cwd(),...parent})+'\\n'); if (${!!opts.unsupportedGit} && a.some(x=>x==='--no-lazy-fetch')) { console.error('unknown option: --no-lazy-fetch'); process.exit(129); } if(a.includes('ls-remote')) { console.log('ref: refs/heads/main\\tHEAD\\n${f.tip}\\tHEAD\\n${f.tip}\\trefs/heads/main'); process.exit(0); } // The fixture remote is already current. Record fetch attempts without contacting a real repository. @@ -758,9 +775,10 @@ export function seedOpportunitySources(f: SharedLibsFixture): void { export function standaloneInstructions(f: SharedLibsFixture, codex = false): string { const source = codex ? path.join(SHARED_LIBS_ROOT, '.agents/skills/gstack-deslop-shared-libs') : path.join(SHARED_LIBS_ROOT, 'deslop-shared-libs'); + // Resolve the installed helper to this checkout, as the hermetic runtime for the skill under test. const text = extractSkillSections(source, [ 'Scope and read-only boundary', 'Establish the reviewed source', 'Start with recent work', 'Evaluate candidates', 'Output', - ]); + ]).replaceAll(codex ? '~/.codex/skills/gstack' : '~/.claude/skills/gstack', SHARED_LIBS_ROOT); const file = path.join(f.root, 'standalone-instructions.md'); fs.writeFileSync(file, text); return file; diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index b87fcf7d1..43ba89019 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -42,14 +42,14 @@ export const E2E_TOUCHFILES: Record = { 'shared-libs-review-path-eligibility': ['review/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/review.ts', 'scripts/resolvers/review-army.ts', 'lib/review-evidence.ts', 'bin/gstack-review-log', 'bin/gstack-review-read', 'bin/gstack-wtree', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs-paths.test.ts', 'test/helpers/shared-libs-path-fixture.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/agent-sdk-runner.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-index-flags-*.json', 'test/fixtures/shared-libs-resolved-reads-public.json', 'test/helpers/shared-libs-review-start-evidence.ts'], 'shared-libs-review-index-flags': ['review/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/review.ts', 'scripts/resolvers/review-army.ts', 'lib/review-evidence.ts', 'bin/gstack-review-log', 'bin/gstack-review-read', 'bin/gstack-wtree', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs-paths.test.ts', 'test/helpers/shared-libs-path-fixture.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/agent-sdk-runner.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-index-flags-*.json', 'test/fixtures/shared-libs-paths-max-turns-public.json', 'test/fixtures/shared-libs-resolved-reads-public.json', 'test/helpers/shared-libs-review-start-evidence.ts'], 'shared-libs-review-prior-coverage': ['review/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/review.ts', 'scripts/resolvers/review-army.ts', 'lib/review-evidence.ts', 'bin/gstack-review-log', 'bin/gstack-review-read', 'bin/gstack-wtree', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs-paths.test.ts', 'test/helpers/shared-libs-path-fixture.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/agent-sdk-runner.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-index-flags-*.json', 'test/fixtures/shared-libs-resolved-reads-public.json', 'test/helpers/shared-libs-review-start-evidence.ts'], - 'shared-libs-codex-read-only': ['deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/helpers/codex-session-runner.ts', 'test/helpers/skill-fixture.ts', 'test/helpers/hermetic-env.ts', 'test/helpers/eval-budgets.ts', 'test/codex-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'hosts/codex.ts', 'hosts/define-host.ts', 'scripts/resolvers/constants.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts'], + 'shared-libs-codex-read-only': ['deslop-shared-libs/**', 'bin/gstack-safe-git', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/helpers/codex-session-runner.ts', 'test/helpers/skill-fixture.ts', 'test/helpers/hermetic-env.ts', 'test/helpers/eval-budgets.ts', 'test/codex-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'hosts/codex.ts', 'hosts/define-host.ts', 'scripts/resolvers/constants.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts'], // Shared-code audit and scoped review lifecycle - 'shared-libs-read-only': ['deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-review-start-evidence.ts', 'test/helpers/shared-libs-path-fixture.ts'], - 'shared-libs-unsupported-git': ['deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-review-start-evidence.ts', 'test/helpers/shared-libs-path-fixture.ts'], + 'shared-libs-read-only': ['deslop-shared-libs/**', 'bin/gstack-safe-git', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-review-start-evidence.ts', 'test/helpers/shared-libs-path-fixture.ts'], + 'shared-libs-unsupported-git': ['deslop-shared-libs/**', 'bin/gstack-safe-git', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-review-start-evidence.ts', 'test/helpers/shared-libs-path-fixture.ts'], 'shared-libs-review-lifecycle': ['deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'review/**', 'scripts/resolvers/review.ts', 'scripts/resolvers/review-army.ts', 'lib/review-evidence.ts', 'bin/gstack-review-log', 'bin/gstack-review-read', 'bin/gstack-wtree', 'test/skill-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/agent-sdk-runner.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-index-flags-*.json', 'test/helpers/shared-libs-path-fixture.ts', 'test/fixtures/shared-libs-lifecycle-r59-stage-scope-public.json', 'test/helpers/shared-libs-review-start-evidence.ts'], 'shared-libs-review-revalidation': ['deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'review/**', 'scripts/resolvers/review.ts', 'scripts/resolvers/review-army.ts', 'lib/review-evidence.ts', 'bin/gstack-review-log', 'bin/gstack-review-read', 'bin/gstack-wtree', 'test/skill-e2e-shared-libs.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/agent-sdk-runner.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/helpers/shared-libs-review-start-evidence.ts', 'test/fixtures/shared-libs-review-start-public.json', 'test/fixtures/shared-libs-revalidation-max-turns-public.json', 'test/fixtures/shared-libs-index-flags-*.json', 'test/helpers/shared-libs-path-fixture.ts' ], - 'shared-libs-opportunity-judgment': ['deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs-periodic.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/llm-judge.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-plan-actor.ts', 'test/helpers/shared-libs-plan-excerpt.ts'], - 'shared-libs-pr-coverage': ['deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs-periodic.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/llm-judge.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-plan-actor.ts', 'test/helpers/shared-libs-plan-excerpt.ts'], + 'shared-libs-opportunity-judgment': ['deslop-shared-libs/**', 'bin/gstack-safe-git', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs-periodic.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/llm-judge.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-plan-actor.ts', 'test/helpers/shared-libs-plan-excerpt.ts'], + 'shared-libs-pr-coverage': ['deslop-shared-libs/**', 'bin/gstack-safe-git', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'test/skill-e2e-shared-libs-periodic.test.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/llm-judge.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts', 'test/fixtures/shared-libs-readonly-substitution-ci16358.json', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/shared-libs-plan-actor.ts', 'test/helpers/shared-libs-plan-excerpt.ts'], 'shared-libs-plan-callers': ['test/helpers/shared-libs-plan-actor.ts', 'scripts/resolvers/confidence.ts', 'test/helpers/shared-libs-plan-excerpt.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'deslop-shared-libs/**', 'scripts/resolvers/shared-libs.ts', 'scripts/resolvers/index.ts', 'test/helpers/shared-libs-eval-fixture.ts', 'plan-eng-review/**', 'test/skill-e2e-shared-libs-periodic.test.ts', 'test/fixtures/plan-scope-recovery-av.json', 'scripts/resolvers/preamble/generate-preamble-bash.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/e2e-gate.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/llm-judge.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts'], 'ship-docsync-missing-marker': ['ship/**', 'document-release/**', 'scripts/resolvers/sections.ts', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/testing.ts', 'scripts/gen-skill-docs.ts', 'bin/gstack-skill-start', 'bin/gstack-session-kind', 'test/helpers/docsync-*.ts', 'test/helpers/qa-functional-*.ts', 'test/helpers/session-runner.ts', 'test/helpers/hermetic-env.ts', 'test/skill-e2e-ship-docsync.test.ts', 'test/helpers/qa-checkpoint-evidence.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/qa-evidence-producer.ts'], 'ship-docsync-missing-asset': ['ship/**', 'document-release/**', 'scripts/resolvers/sections.ts', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/testing.ts', 'scripts/gen-skill-docs.ts', 'bin/gstack-skill-start', 'bin/gstack-session-kind', 'test/helpers/docsync-*.ts', 'test/helpers/qa-functional-*.ts', 'test/helpers/session-runner.ts', 'test/helpers/hermetic-env.ts', 'test/skill-e2e-ship-docsync.test.ts', 'test/helpers/qa-checkpoint-evidence.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/qa-evidence-producer.ts'], diff --git a/test/shared-libs-fixture.test.ts b/test/shared-libs-fixture.test.ts index de15cb2d4..d62f37840 100644 --- a/test/shared-libs-fixture.test.ts +++ b/test/shared-libs-fixture.test.ts @@ -6,7 +6,7 @@ import * as path from 'node:path'; import { execFileSync, spawnSync } from 'node:child_process'; import { createSharedInteractiveToolHandler, createSharedLibsFixture, fixtureGit, fixtureWrite, installSourceShims, - readRequests, seedOpportunitySources, sharedReadOnlyViolations, shellQuote, snapshotFixture, type SharedLibsFixture, + installHostileGitConfig, isGuardedGitRequest, standaloneInstructions, SHARED_LIBS_ROOT, readRequests, seedOpportunitySources, sharedReadOnlyViolations, shellQuote, snapshotFixture, type SharedLibsFixture, SharedCaptureAccumulator, type SharedCaptureAttempt, isInternalClaudeGitRequest, SHARED_LIBS_OLDER_OPEN_PRS, incompleteFirstFileView, } from './helpers/shared-libs-eval-fixture'; import { EvalCollector, type EvalTestEntry } from './helpers/eval-store'; @@ -27,6 +27,51 @@ function scratch(): string { return directory; } +describe('shared-code Git guard', () => { + test('the standalone runtime resolves the real helper, and only its complete prefix counts as a guarded read', () => { + const f = createSharedLibsFixture('safe-git'); + cleanup.push(f.root); + seedOpportunitySources(f); + installHostileGitConfig(f); + installSourceShims(f); + const instructions = fs.readFileSync(standaloneInstructions(f), 'utf8'); + const helper = path.join(SHARED_LIBS_ROOT, 'bin/gstack-safe-git'); + expect(instructions).toContain(`\`${helper} rev-parse --is-inside-work-tree\``); + expect(instructions).not.toContain('~/.claude/skills/gstack'); + const run = (command: string, args: string[]) => spawnSync(command, args, { + cwd: f.repo, encoding: 'utf8', timeout: 10_000, env: { ...process.env, ...f.env } }); + const refusal = run(helper, ['status']); + expect(refusal.status).toBe(2); + expect(refusal.stderr).toContain('gstack-safe-git: refused'); + // Exit status is Git's own; with Git < 2.44 these reach the recording shim and then fail on --no-lazy-fetch. + for (const args of [['rev-parse', '--is-inside-work-tree'], ['log', '-p', '-1'], + ['diff', f.tip, f.tip, '--', 'README.md'], ['ls-files', '--cached', '--others', '--exclude-standard', '-z']]) run(helper, args); + const guarded = readRequests(f).filter(row => row.tool === 'git'); + expect(guarded.map(row => row.args[9])).toEqual(['rev-parse', 'log', 'diff', 'ls-files']); + for (const row of guarded) expect(isGuardedGitRequest(row), JSON.stringify(row)).toBe(true); + expect(fs.existsSync(f.hookTrace) ? fs.readFileSync(f.hookTrace, 'utf8') : '').toBe(''); + + fs.rmSync(f.trace, { force: true }); + run('git', ['log', '-1']); + run('git', ['--no-lazy-fetch', '-c', 'core.fsmonitor=false', '-c', 'log.showSignature=false', 'rev-parse', 'HEAD']); + const unguarded = readRequests(f); + expect(unguarded).toHaveLength(2); + for (const row of unguarded) expect(isGuardedGitRequest(row), JSON.stringify(row)).toBe(false); + + const wrapped = guarded[2]!; + const without = (value: string) => ({ ...wrapped, args: wrapped.args.filter(arg => arg !== value) }); + for (const damaged of [ + { ...wrapped, env: {} }, + { ...wrapped, env: { ...wrapped.env, GIT_NO_LAZY_FETCH: '0' } }, + without('--no-replace-objects'), without('--no-pager'), without('diff.submodule=short'), + without('--no-textconv'), without('--no-ext-diff'), + { ...wrapped, args: [...wrapped.args, '--ext-diff'] }, + { ...wrapped, args: [...wrapped.args, '--output=out.patch'] }, + { ...wrapped, args: wrapped.args.map(arg => arg === 'core.fsmonitor=false' ? 'core.fsmonitor=true' : arg) }, + ]) expect(isGuardedGitRequest(damaged), JSON.stringify(damaged.args)).toBe(false); + }); +}); + describe('shared-code legacy interactive actor', () => { test('R58 acknowledges the exact removed-filter Skip packet without authorizing its recommended fix', async () => { const input = { diff --git a/test/shared-libs-rendering.test.ts b/test/shared-libs-rendering.test.ts index f650f54db..c750ecc75 100644 --- a/test/shared-libs-rendering.test.ts +++ b/test/shared-libs-rendering.test.ts @@ -54,8 +54,14 @@ describe('shared-code skill distribution', () => { expect(standalone).toContain('including repeated page numbers'); expect(standalone).toContain('temporary files and files outside the repository'); expect(standalone).toContain('Keep API responses and intermediate data on stdout or in memory'); - expect(standalone).toContain('--no-lazy-fetch'); - expect(standalone).toContain('log.showSignature=false'); + // Git safety is the installed helper from the trusted global runtime, not a retyped prefix. + const safeGit = `~/${host.globalRoot}/bin/gstack-safe-git`; + expect(standalone).toContain(`${safeGit} rev-parse --is-inside-work-tree`); + expect(standalone).toContain(`${safeGit} ls-files --cached --others --exclude-standard -z`); + expect(standalone).toContain('never bare `git`'); + expect(standalone).not.toContain('git --no-pager'); + expect(standalone).not.toContain('$GSTACK_ROOT'); + expect(standalone).not.toContain('{{SAFE_GIT}}'); expect(standalone).toContain('python3 -I -S'); expect(standalone).toContain('two explicit committed object IDs'); expect(standalone).toContain('same Git tree'); diff --git a/test/skill-e2e-shared-libs.test.ts b/test/skill-e2e-shared-libs.test.ts index 268a4aa15..7eef966d4 100644 --- a/test/skill-e2e-shared-libs.test.ts +++ b/test/skill-e2e-shared-libs.test.ts @@ -11,7 +11,7 @@ import { EvalCollector } from './helpers/eval-store'; import { addBranchAndRawOverlay, createSharedLibsFixture, fixtureGit, fixtureWrite, fixtureWorkingTree, installHostileGitConfig, installInterpreterCanary, installNormalizingFilter, - isInternalClaudeGitRequest, + isGuardedGitRequest, isInternalClaudeGitRequest, installSourceShims, readRequests, reviewLifecycleInstructions, reviewPrompt, reviewRevalidationPrompt, reviewRecords, runSharedCapture, runSharedInteractive, seedOpportunitySources, seedReviewSources, seedSkippedAdvisory, snapshotFixture, specialistFixture, @@ -88,9 +88,8 @@ function assertReadOnly(f: SharedLibsFixture, before: Record, re expect(request.args).not.toContain('add'); expect(request.args).not.toContain('write-tree'); expect(request.args).not.toContain('hash-object'); - expect(request.args).toContain('--no-lazy-fetch'); - expect(request.args).toContain('core.fsmonitor=false'); - expect(request.args).toContain('log.showSignature=false'); + // Every repository read carries the complete gstack-safe-git prefix, environment included. + expect(isGuardedGitRequest(request), JSON.stringify(request)).toBe(true); } for (const request of requests.filter(row => row.tool === 'gh' && row.args[0] === 'api')) { expect(request.method).toBe('GET');