mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-30 03:52:20 +00:00
fix: repair the Windows test lane's standing failures (#10064)
* fix: repair the Windows test lane's standing failures
Fixes the eight failure clusters that keep Test (windows-latest) red on
the ECS nightly lane — three product bugs and five test bugs.
Product:
- extension-git-client: GIT_CONFIG_GLOBAL was os.devNull, whose win32
spelling ('\\.\nul') Git for Windows rejects with "Invalid argument",
failing every restricted-env git command on Windows. Use the literal
'/dev/null', which git's compat layer maps on every platform.
- sessionArtifacts: the pre-open/post-open identity check compared
numeric dev/ino. NTFS file ids are 64-bit and lose precision above
2^53 as Numbers, so two files created close together can round to the
same ino and defeat the swap (TOCTOU) detection. Compare bigint stats.
- dws-event-stream: clearing a stale stderr error on a healthy stdout
line was gated on the child still being alive, but on Windows the pipe
routinely drains only after exitCode is set, so the final healthy line
never cleared it and close reported the stale error. A clean (code 0)
exit now certifies recorded errors as stale at close.
Tests:
- daemon-git-worktree-guard: fixtures pasted native Windows paths into
command strings unquoted; on the bash lane the tokenizer consumes the
backslashes (C:\t\repo -> C:trepo), so resolvable targets tested as
unresolvable ones and cd shifts silently failed in place. Interpolate
through cmdPath(), which spells them with forward slashes on win32 —
what bash preserves and Windows resolves. Assertions keep the native
spelling; the guard canonicalizes targets through realpath.
- review/lib/worktree: git marks a linked worktree's .git gitfile hidden
on Windows and CREATE_ALWAYS on a hidden file fails with EPERM;
rewrite gitfiles via unlink-then-write.
- config: compare getCwd() against path.resolve(targetDir) — the
constructor resolves it, so the POSIX fixture comes back
drive-qualified on win32.
- SessionMessageHandler: complete the pathToFileURL shim's mirror case —
parse POSIX-spelled fixture paths with { windows: false } on win32 so
expected file:// URLs read the same on every host.
- node-repl.semantics: dispose the kernel before removing its cwd (a
process's cwd cannot be removed on Windows) and retry the removal
across the child's asynchronous teardown.
* fix: witness the Windows-lane fixes per review feedback (#10064)
- Document why the workspace-status identity fstat cannot be folded into
the Number stat (bigint stats truncate mtimeMs and return BigInt fields).
- Add a clean-exit fixture + test proving the dws consumer reports the
code-0 process error instead of a stale stderr error.
- Add an injected-stats test detecting swaps whose file ids differ only
above 2^53, so a numeric identity revert cannot ship green.
- Route the remaining unquoted/eval'd env-var assignment payloads in the
worktree guard tests through cmdPath so they stay resolvable on the
win32+bash lane.
* test(acp-bridge): consolidate the duplicated forwarding lstat spy (#10064)
---------
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
This commit is contained in:
Shaojin Wen
GitHub
parent
4167d4f68e
commit
3a46420d59
11 changed files with 284 additions and 64 deletions
|
|
@ -6,7 +6,7 @@
|
|||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { promises as fs } from 'node:fs';
|
||||
import { promises as fs, type BigIntStats, type Stats } from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
|
|
@ -46,6 +46,30 @@ describe('SessionArtifactStore', () => {
|
|||
.slice(0, 16);
|
||||
}
|
||||
|
||||
// Forward the options through: the store lstats with { bigint: true }
|
||||
// for the identity check, and a spy that dropped it would hand back
|
||||
// numeric stats that never equal the bigint fstat — hiding a broken
|
||||
// comparison behind a type mismatch.
|
||||
function spyOnLstatForwarding(
|
||||
hook: (
|
||||
entry: Parameters<typeof fs.lstat>[0],
|
||||
stat: Stats | BigIntStats,
|
||||
) => void | Promise<void>,
|
||||
) {
|
||||
const originalLstat = fs.lstat.bind(fs);
|
||||
return vi.spyOn(fs, 'lstat').mockImplementation((async (
|
||||
entry: Parameters<typeof fs.lstat>[0],
|
||||
options?: Parameters<typeof fs.lstat>[1],
|
||||
) => {
|
||||
const stat = await originalLstat(
|
||||
entry,
|
||||
options as Parameters<typeof originalLstat>[1],
|
||||
);
|
||||
await hook(entry, stat);
|
||||
return stat;
|
||||
}) as typeof fs.lstat);
|
||||
}
|
||||
|
||||
it('lists, removes, and idempotently ignores missing artifact deletes', async () => {
|
||||
const store = new SessionArtifactStore({
|
||||
sessionId: 's1',
|
||||
|
|
@ -3244,16 +3268,13 @@ describe('SessionArtifactStore', () => {
|
|||
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date(Date.now() + 6_000));
|
||||
const originalLstat = fs.lstat.bind(fs);
|
||||
let swapped = false;
|
||||
const lstatSpy = vi.spyOn(fs, 'lstat').mockImplementation(async (entry) => {
|
||||
const stat = await originalLstat(entry);
|
||||
const lstatSpy = spyOnLstatForwarding(async (entry) => {
|
||||
if (!swapped && String(entry) === realTarget) {
|
||||
swapped = true;
|
||||
await fs.writeFile(replacement, 'after');
|
||||
await fs.rename(replacement, target);
|
||||
}
|
||||
return stat;
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -3266,6 +3287,89 @@ describe('SessionArtifactStore', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('detects a swap whose file ids differ only above 2^53', async () => {
|
||||
const store = new SessionArtifactStore({
|
||||
sessionId: 's7-bigint-identity',
|
||||
workspaceCwd: workspace,
|
||||
});
|
||||
await fs.writeFile(path.join(workspace, 'big.txt'), 'content');
|
||||
const created = await store.upsertMany([
|
||||
{ title: 'Big', workspacePath: 'big.txt' },
|
||||
]);
|
||||
const artifactId = created.changes[0]!.artifactId;
|
||||
const realTarget = await fs.realpath(path.join(workspace, 'big.txt'));
|
||||
|
||||
// Injected ids on both sides of 2^53: the replacement rounds to the SAME
|
||||
// Number as the original, so only the bigint comparison can see the swap.
|
||||
const preOpenIno = 2n ** 53n;
|
||||
const injectIno = (stat: Stats | BigIntStats, ino: bigint): void => {
|
||||
stat.ino = typeof stat.ino === 'bigint' ? ino : Number(ino);
|
||||
};
|
||||
const lstatSpy = spyOnLstatForwarding((entry, stat) => {
|
||||
if (String(entry) === realTarget) {
|
||||
injectIno(stat, preOpenIno);
|
||||
}
|
||||
});
|
||||
const originalOpen = fs.open.bind(fs);
|
||||
const openSpy = vi.spyOn(fs, 'open').mockImplementation((async (
|
||||
entry: Parameters<typeof fs.open>[0],
|
||||
flags?: Parameters<typeof fs.open>[1],
|
||||
mode?: Parameters<typeof fs.open>[2],
|
||||
) => {
|
||||
const handle = await originalOpen(
|
||||
entry,
|
||||
flags as Parameters<typeof originalOpen>[1],
|
||||
mode,
|
||||
);
|
||||
if (String(entry) !== realTarget) {
|
||||
return handle;
|
||||
}
|
||||
const originalStat = handle.stat.bind(handle);
|
||||
handle.stat = (async (options?: Parameters<typeof handle.stat>[0]) => {
|
||||
const stat = await originalStat(options);
|
||||
injectIno(stat, preOpenIno + 1n);
|
||||
return stat;
|
||||
}) as typeof handle.stat;
|
||||
return handle;
|
||||
}) as typeof fs.open);
|
||||
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date(Date.now() + 6_000));
|
||||
|
||||
try {
|
||||
const artifact = await store.get(artifactId);
|
||||
expect(artifact).toMatchObject({ id: artifactId, status: 'missing' });
|
||||
} finally {
|
||||
lstatSpy.mockRestore();
|
||||
openSpy.mockRestore();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps an unswapped file available through an options-forwarding lstat spy', async () => {
|
||||
const store = new SessionArtifactStore({
|
||||
sessionId: 's7-bigint-forwarding',
|
||||
workspaceCwd: workspace,
|
||||
});
|
||||
await fs.writeFile(path.join(workspace, 'keep.txt'), 'content');
|
||||
const created = await store.upsertMany([
|
||||
{ title: 'Keep', workspacePath: 'keep.txt' },
|
||||
]);
|
||||
const artifactId = created.changes[0]!.artifactId;
|
||||
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date(Date.now() + 6_000));
|
||||
const lstatSpy = spyOnLstatForwarding(() => {});
|
||||
|
||||
try {
|
||||
const artifact = await store.get(artifactId);
|
||||
expect(artifact).toMatchObject({ id: artifactId, status: 'available' });
|
||||
} finally {
|
||||
lstatSpy.mockRestore();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects workspace paths that become symlinks before open', async () => {
|
||||
const store = new SessionArtifactStore({
|
||||
sessionId: 's7-open-nofollow',
|
||||
|
|
|
|||
|
|
@ -5,7 +5,12 @@
|
|||
*/
|
||||
|
||||
import { createHash } from 'node:crypto';
|
||||
import { constants as fsConstants, promises as fs, type Stats } from 'node:fs';
|
||||
import {
|
||||
constants as fsConstants,
|
||||
promises as fs,
|
||||
type BigIntStats,
|
||||
type Stats,
|
||||
} from 'node:fs';
|
||||
import type { FileHandle } from 'node:fs/promises';
|
||||
import path from 'node:path';
|
||||
import {
|
||||
|
|
@ -3139,16 +3144,21 @@ async function getWorkspaceStatus(
|
|||
if (!relative || isOutsidePath(relative)) {
|
||||
return { status: 'missing', escaped: true };
|
||||
}
|
||||
const preOpenStat = await fs.lstat(realPath);
|
||||
// Identity is compared on bigint stats: NTFS file ids are 64-bit and the
|
||||
// Number spelling loses precision above 2^53, so two files created close
|
||||
// together can round to the SAME numeric ino and defeat the swap check.
|
||||
const preOpenStat = await fs.lstat(realPath, { bigint: true });
|
||||
const handle = await fs.open(
|
||||
realPath,
|
||||
fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW,
|
||||
);
|
||||
try {
|
||||
const stat = await handle.stat();
|
||||
if (!isSameFile(preOpenStat, stat)) {
|
||||
if (!isSameFile(preOpenStat, await handle.stat({ bigint: true }))) {
|
||||
return { status: 'missing', escaped: true };
|
||||
}
|
||||
// The identity fstat cannot be reused here: bigint stats return BigInt
|
||||
// fields and truncate mtimeMs, breaking the Number comparisons below.
|
||||
const stat = await handle.stat();
|
||||
if (!stat.isFile()) {
|
||||
throw new Error('path is not a regular file');
|
||||
}
|
||||
|
|
@ -3219,7 +3229,7 @@ async function getWorkspaceStatus(
|
|||
}
|
||||
}
|
||||
|
||||
function isSameFile(before: Stats, after: Stats): boolean {
|
||||
function isSameFile(before: BigIntStats, after: BigIntStats): boolean {
|
||||
return before.dev === after.dev && before.ino === after.ino;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -81,6 +81,29 @@ describe('DWS event process', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('reports a clean exit instead of a stale stderr error', async () => {
|
||||
const onLine = vi.fn();
|
||||
const onError = vi.fn();
|
||||
const fixture = fileURLToPath(
|
||||
new URL('./fixtures/dws-event-clean-exit-source.mjs', import.meta.url),
|
||||
);
|
||||
const subscription = await startDwsEventProcess(
|
||||
process.execPath,
|
||||
[fixture],
|
||||
onLine,
|
||||
onError,
|
||||
);
|
||||
|
||||
await subscription.closed;
|
||||
|
||||
expect(onLine).toHaveBeenCalledWith('{"type":"final"}');
|
||||
expect(onError).toHaveBeenCalledOnce();
|
||||
expect(onError.mock.calls[0]?.[0]).toMatchObject({
|
||||
message: 'DWS event consumer stopped (0).',
|
||||
retryable: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves a terminal error across stdout buffered after exit', async () => {
|
||||
let releaseFirstLine!: () => void;
|
||||
const firstLineBlocked = new Promise<void>((resolve) => {
|
||||
|
|
|
|||
|
|
@ -216,7 +216,15 @@ export const startDwsEventProcess: DwsEventProcessStarter = (
|
|||
if (state === 'pending') {
|
||||
settleStartupError(lastError ?? processError(code));
|
||||
} else if (state === 'ready' && !stopping) {
|
||||
reportError(lastError ?? processError(code));
|
||||
// A clean exit certifies any recorded stderr error as stale: the
|
||||
// consumer chose to finish after writing it. The live-line clear in
|
||||
// the stdout handler cannot be relied on for the final line — on
|
||||
// Windows the pipe routinely drains only after `exitCode` is set,
|
||||
// so its liveness gate (which protects errors from being cleared
|
||||
// by lines buffered across a crash) also swallows that clear.
|
||||
reportError(
|
||||
code === 0 ? processError(code) : (lastError ?? processError(code)),
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen Team
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import process from 'node:process';
|
||||
|
||||
process.stderr.write('[event] ready\n');
|
||||
process.stdout.write('{"type":"final"}\n', () => {
|
||||
process.stderr.write(
|
||||
'{"message":"stale stderr error","retryable":false}\n',
|
||||
() => process.exit(0),
|
||||
);
|
||||
});
|
||||
|
|
@ -39,6 +39,15 @@ import {
|
|||
worktreeResidue,
|
||||
} from './worktree.js';
|
||||
|
||||
// Replaces a gitfile that `git worktree add` created. On Windows git marks
|
||||
// the linked worktree's `.git` hidden, and opening a hidden file for truncate
|
||||
// (writeFileSync's CREATE_ALWAYS) fails with EPERM — so unlink first and let
|
||||
// the rewrite create a fresh, unhidden file.
|
||||
function overwriteGitfile(gitfilePath: string, content: string): void {
|
||||
rmSync(gitfilePath, { force: true });
|
||||
writeFileSync(gitfilePath, content);
|
||||
}
|
||||
|
||||
describe('worktreeResidue', () => {
|
||||
let repo: string;
|
||||
// The tree under measurement is a LINKED worktree — the production shape:
|
||||
|
|
@ -245,7 +254,7 @@ describe('worktreeResidue', () => {
|
|||
]);
|
||||
|
||||
gitRepo('config', 'core.worktree', tree);
|
||||
writeFileSync(join(tree, '.git'), `gitdir: ${join(repo, '.git')}\n`);
|
||||
overwriteGitfile(join(tree, '.git'), `gitdir: ${join(repo, '.git')}\n`);
|
||||
|
||||
const got = worktreeResidue(tree);
|
||||
|
||||
|
|
@ -296,7 +305,7 @@ describe('worktreeResidue', () => {
|
|||
writeFileSync(join(admin, 'commondir'), '../..\n');
|
||||
writeFileSync(join(admin, 'HEAD'), `${forgedHead}\n`);
|
||||
copyFileSync(join(repo, '.git', 'index'), join(admin, 'index'));
|
||||
writeFileSync(join(tree, '.git'), `gitdir: ${admin}\n`);
|
||||
overwriteGitfile(join(tree, '.git'), `gitdir: ${admin}\n`);
|
||||
|
||||
// Unpinned, the forge's index answers clean — and an unanchored clean
|
||||
// verdict is exactly the one the probe refuses (#9557).
|
||||
|
|
@ -333,7 +342,7 @@ describe('worktreeResidue', () => {
|
|||
const admin = readFileSync(join(sibling, '.git'), 'utf8')
|
||||
.trim()
|
||||
.replace(/^gitdir:\s*/, '');
|
||||
writeFileSync(join(tree, '.git'), `gitdir: ${admin}\n`);
|
||||
overwriteGitfile(join(tree, '.git'), `gitdir: ${admin}\n`);
|
||||
|
||||
const got = worktreeResidue(tree);
|
||||
|
||||
|
|
@ -468,7 +477,7 @@ describe('worktreeResidue', () => {
|
|||
// absolutely, because its old relative spelling no longer resolves from
|
||||
// outside the repo.
|
||||
renameSync(tree, join(outside, 'review-wt'));
|
||||
writeFileSync(
|
||||
overwriteGitfile(
|
||||
join(outside, 'review-wt', '.git'),
|
||||
`gitdir: ${join(repo, '.git', 'worktrees', 'review-wt')}\n`,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -39,6 +39,16 @@ const outsideRepo = path.join(temporaryRoot, 'outside', 'repo');
|
|||
// A second outside checkout whose path contains no Git word, so a test using
|
||||
// it cannot pass because `\bgit\b` happened to match inside the path.
|
||||
const plainOutsidePath = path.join(temporaryRoot, 'elsewhere', 'checkout');
|
||||
// Spelling for paths interpolated into COMMAND STRINGS on the lanes that
|
||||
// execute through bash: an unquoted backslash is a POSIX escape, so a native
|
||||
// Windows path pasted into a command reaches the tokenizer as `C:temprepo`
|
||||
// and the test stops exercising what it names — resolvable targets become
|
||||
// unresolvable ones, and cwd shifts silently fail in place. Forward slashes
|
||||
// survive bash and resolve identically on Windows. Filesystem setup and
|
||||
// denial-reason assertions keep the native spelling: the guard canonicalizes
|
||||
// targets through realpath before printing them.
|
||||
const cmdPath = (value: string): string =>
|
||||
process.platform === 'win32' ? value.replaceAll('\\', '/') : value;
|
||||
mkdirSync(path.join(outsideRepo, '.git'), { recursive: true });
|
||||
mkdirSync(insideNested, { recursive: true });
|
||||
|
||||
|
|
@ -68,17 +78,19 @@ afterAll(async () => {
|
|||
|
||||
describe('createDaemonToolGuard', () => {
|
||||
it.each([
|
||||
() => `git -C ${outsideRepo} reset --hard`,
|
||||
() => `git -C${outsideRepo} checkout -- .`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} reset --hard`,
|
||||
() => `git -C${cmdPath(outsideRepo)} checkout -- .`,
|
||||
() =>
|
||||
`git --work-tree=${outsideRepo} --git-dir=${path.join(outsideRepo, '.git')} clean -fd`,
|
||||
() => `git --git-dir ${path.join(outsideRepo, '.git')} commit -m x`,
|
||||
() => `git --namespace foo -C ${outsideRepo} reset --hard`,
|
||||
() => `git --super-prefix=foo --work-tree=${outsideRepo} clean -fd`,
|
||||
`git --work-tree=${cmdPath(outsideRepo)} --git-dir=${cmdPath(path.join(outsideRepo, '.git'))} clean -fd`,
|
||||
() =>
|
||||
`git --git-dir ${cmdPath(path.join(outsideRepo, '.git'))} commit -m x`,
|
||||
() => `git --namespace foo -C ${cmdPath(outsideRepo)} reset --hard`,
|
||||
() =>
|
||||
`git --super-prefix=foo --work-tree=${cmdPath(outsideRepo)} clean -fd`,
|
||||
// `grep` runs the target repo's diff.<driver>.textconv programs and
|
||||
// `status` refreshes the target index + runs its core.fsmonitor.
|
||||
() => `git -C ${outsideRepo} grep --textconv pattern`,
|
||||
() => `git -C ${outsideRepo} status --porcelain`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} grep --textconv pattern`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} status --porcelain`,
|
||||
])('denies relocated mutating Git command %#', async (buildCommand) => {
|
||||
const guard = createDaemonToolGuard();
|
||||
|
||||
|
|
@ -171,7 +183,7 @@ describe('createDaemonToolGuard', () => {
|
|||
|
||||
it.each([
|
||||
// Git treats an empty `-C` as a no-op and applies the next relocation.
|
||||
() => `git -C "" -C ${outsideRepo} reset --hard`,
|
||||
() => `git -C "" -C ${cmdPath(outsideRepo)} reset --hard`,
|
||||
])('denies relocations masked by token edge cases %#', async (command) => {
|
||||
const guard = createDaemonToolGuard();
|
||||
|
||||
|
|
@ -185,7 +197,7 @@ describe('createDaemonToolGuard', () => {
|
|||
// is ordinary argv text and the divergent-syntax gate owns the shape.
|
||||
it
|
||||
.runIf(bashSemanticsLane)
|
||||
.each([() => `git -C ${outsideRepo} reset --hard # note`])(
|
||||
.each([() => `git -C ${cmdPath(outsideRepo)} reset --hard # note`])(
|
||||
'denies relocations masked by token edge cases on the bash lanes %#',
|
||||
async (command) => {
|
||||
const guard = createDaemonToolGuard();
|
||||
|
|
@ -412,7 +424,9 @@ describe('createDaemonToolGuard', () => {
|
|||
const guard = createDaemonToolGuard();
|
||||
|
||||
await expect(
|
||||
guard(request(`cd ${effectiveCwd} && sh -c 'git reset --hard'`)),
|
||||
guard(
|
||||
request(`cd ${cmdPath(effectiveCwd)} && sh -c 'git reset --hard'`),
|
||||
),
|
||||
).resolves.toEqual({ allowed: true });
|
||||
},
|
||||
);
|
||||
|
|
@ -491,17 +505,17 @@ describe('createDaemonToolGuard', () => {
|
|||
guard(request(`sh -c 'cd ${outsideRepo}'; git reset --hard`)),
|
||||
).resolves.toEqual({ allowed: true });
|
||||
await expect(
|
||||
guard(request(`cd ${effectiveCwd} && git reset --hard`)),
|
||||
guard(request(`cd ${cmdPath(effectiveCwd)} && git reset --hard`)),
|
||||
).resolves.toEqual({ allowed: true });
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(bashSemanticsLane).each([
|
||||
() => `git -C \\
|
||||
${outsideRepo} reset --hard`,
|
||||
${cmdPath(outsideRepo)} reset --hard`,
|
||||
|
||||
() => `g\\
|
||||
it -C ${outsideRepo} reset --hard`,
|
||||
it -C ${cmdPath(outsideRepo)} reset --hard`,
|
||||
])(
|
||||
'joins backslash continuations before parsing %#',
|
||||
async (buildCommand) => {
|
||||
|
|
@ -608,12 +622,12 @@ it -C ${outsideRepo} reset --hard`,
|
|||
const guard = createDaemonToolGuard();
|
||||
|
||||
await expect(
|
||||
guard(request(`git -C nested -C ${outsideRepo} reset --hard`)),
|
||||
guard(request(`git -C nested -C ${cmdPath(outsideRepo)} reset --hard`)),
|
||||
).resolves.toMatchObject({ allowed: false });
|
||||
await expect(
|
||||
guard(
|
||||
request(
|
||||
`git -C ${outsideRepo} -C ${path.relative(outsideRepo, effectiveCwd)} reset --hard`,
|
||||
`git -C ${cmdPath(outsideRepo)} -C ${cmdPath(path.relative(outsideRepo, effectiveCwd))} reset --hard`,
|
||||
),
|
||||
),
|
||||
).resolves.toEqual({ allowed: true });
|
||||
|
|
@ -1546,11 +1560,12 @@ it -C ${outsideRepo} reset --hard`,
|
|||
// An unrecognized program word hides what runs, so a git mention only
|
||||
// survives while the shell is provably still inside the boundary.
|
||||
it.each([
|
||||
() => `cd ${outsideRepo} && nice git reset --hard`,
|
||||
() => `cd ${outsideRepo} && ionice -c3 git reset --hard`,
|
||||
() => `cd ${outsideRepo} && echo x | xargs -I{} git reset --hard`,
|
||||
() => `cd ${outsideRepo} && find . -maxdepth 0 -exec git reset --hard ;`,
|
||||
() => `cd ${outsideRepo} && stdbuf -o0 git reset --hard`,
|
||||
() => `cd ${cmdPath(outsideRepo)} && nice git reset --hard`,
|
||||
() => `cd ${cmdPath(outsideRepo)} && ionice -c3 git reset --hard`,
|
||||
() => `cd ${cmdPath(outsideRepo)} && echo x | xargs -I{} git reset --hard`,
|
||||
() =>
|
||||
`cd ${cmdPath(outsideRepo)} && find . -maxdepth 0 -exec git reset --hard ;`,
|
||||
() => `cd ${cmdPath(outsideRepo)} && stdbuf -o0 git reset --hard`,
|
||||
() => 'cd - && nice git reset --hard',
|
||||
])(
|
||||
'denies an unrecognized program running Git after a cwd shift %#',
|
||||
|
|
@ -1605,7 +1620,11 @@ it -C ${outsideRepo} reset --hard`,
|
|||
guard(request('export FOO=bar && git commit -m x')),
|
||||
).resolves.toEqual({ allowed: true });
|
||||
await expect(
|
||||
guard(request(`export GIT_WORK_TREE=${insideNested} && git commit -m x`)),
|
||||
guard(
|
||||
request(
|
||||
`export GIT_WORK_TREE=${cmdPath(insideNested)} && git commit -m x`,
|
||||
),
|
||||
),
|
||||
).resolves.toEqual({ allowed: true });
|
||||
});
|
||||
|
||||
|
|
@ -1666,11 +1685,14 @@ it -C ${outsideRepo} reset --hard`,
|
|||
// The relocated read-only allowance covers subcommands that neither write
|
||||
// files nor run target-repository programs — flags can revoke both.
|
||||
it.each([
|
||||
() => `git -C ${outsideRepo} cat-file --textconv --path=f.txt HEAD:f.txt`,
|
||||
() => `git -C ${outsideRepo} cat-file --filters --path=f.txt HEAD:f.txt`,
|
||||
() => `git -C ${outsideRepo} rev-parse --output=${outsideRepo}/o.txt HEAD`,
|
||||
() =>
|
||||
`git -C ${outsideRepo} cat-file --output ${outsideRepo}/o.txt -p HEAD`,
|
||||
`git -C ${cmdPath(outsideRepo)} cat-file --textconv --path=f.txt HEAD:f.txt`,
|
||||
() =>
|
||||
`git -C ${cmdPath(outsideRepo)} cat-file --filters --path=f.txt HEAD:f.txt`,
|
||||
() =>
|
||||
`git -C ${cmdPath(outsideRepo)} rev-parse --output=${cmdPath(outsideRepo)}/o.txt HEAD`,
|
||||
() =>
|
||||
`git -C ${cmdPath(outsideRepo)} cat-file --output ${cmdPath(outsideRepo)}/o.txt -p HEAD`,
|
||||
])(
|
||||
'denies a relocated read-only subcommand carrying a disqualifying flag %#',
|
||||
async (buildCommand) => {
|
||||
|
|
@ -1697,8 +1719,8 @@ it -C ${outsideRepo} reset --hard`,
|
|||
// `ls-files` executes the target repository's core.fsmonitor hook — the
|
||||
// same property that excluded `status` (measured on git 2.47.3).
|
||||
it.each([
|
||||
() => `git -C ${outsideRepo} ls-files`,
|
||||
() => `git -C ${outsideRepo} ls-files --others`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} ls-files`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} ls-files --others`,
|
||||
])('denies a relocated ls-files %#', async (buildCommand) => {
|
||||
const guard = createDaemonToolGuard();
|
||||
|
||||
|
|
@ -1714,11 +1736,11 @@ it -C ${outsideRepo} reset --hard`,
|
|||
// subcommand stays out of the read-only set because the flag is one token
|
||||
// away from any describe a model writes.
|
||||
it.each([
|
||||
() => `git -C ${outsideRepo} describe`,
|
||||
() => `git -C ${outsideRepo} describe --tags`,
|
||||
() => `git -C ${outsideRepo} describe --dirty`,
|
||||
() => `git -C ${outsideRepo} describe --always --dirty`,
|
||||
() => `git -C ${outsideRepo} describe --broken`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} describe`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} describe --tags`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} describe --dirty`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} describe --always --dirty`,
|
||||
() => `git -C ${cmdPath(outsideRepo)} describe --broken`,
|
||||
])('denies a relocated describe %#', async (buildCommand) => {
|
||||
const guard = createDaemonToolGuard();
|
||||
|
||||
|
|
@ -1752,7 +1774,7 @@ it -C ${outsideRepo} reset --hard`,
|
|||
() => `xargs -I{} sh -c 'cd ${outsideRepo} && git reset --hard'`,
|
||||
// `executableBaseName` lowercases, so an uppercase program word resolves
|
||||
// to the same binary on a case-insensitive filesystem.
|
||||
() => `cd ${outsideRepo} && nice GIT reset --hard`,
|
||||
() => `cd ${cmdPath(outsideRepo)} && nice GIT reset --hard`,
|
||||
])(
|
||||
'denies a relocated mutation concealed in an unrecognized program %#',
|
||||
async (buildCommand) => {
|
||||
|
|
@ -1766,8 +1788,8 @@ it -C ${outsideRepo} reset --hard`,
|
|||
|
||||
// A program word the daemon cannot read is as opaque as an unrecognized one.
|
||||
it.each([
|
||||
() => `cd ${outsideRepo} && $CMD git reset --hard`,
|
||||
() => `cd ${outsideRepo} && command $CMD git reset --hard`,
|
||||
() => `cd ${cmdPath(outsideRepo)} && $CMD git reset --hard`,
|
||||
() => `cd ${cmdPath(outsideRepo)} && command $CMD git reset --hard`,
|
||||
])(
|
||||
'denies a dynamic program running Git after a cwd shift %#',
|
||||
async (buildCommand) => {
|
||||
|
|
@ -1840,8 +1862,9 @@ it -C ${outsideRepo} reset --hard`,
|
|||
// An unmodelled value-taking global option makes its value look like the
|
||||
// subcommand, which ends option parsing and hides the relocation after it.
|
||||
it.each([
|
||||
() => `git --shallow-file /tmp/shallow -C ${outsideRepo} reset --hard`,
|
||||
() => `git --attr-source HEAD -C ${outsideRepo} reset --hard`,
|
||||
() =>
|
||||
`git --shallow-file /tmp/shallow -C ${cmdPath(outsideRepo)} reset --hard`,
|
||||
() => `git --attr-source HEAD -C ${cmdPath(outsideRepo)} reset --hard`,
|
||||
])(
|
||||
'parses relocations after value-taking global options %#',
|
||||
async (buildCommand) => {
|
||||
|
|
@ -2030,11 +2053,12 @@ it -C ${outsideRepo} reset --hard`,
|
|||
|
||||
// Values assigned earlier in the same command are visible to the guard.
|
||||
it.each([
|
||||
() => `X='git reset --hard'; cd ${outsideRepo}; $X`,
|
||||
() => `X='git reset --hard'; cd ${cmdPath(outsideRepo)}; $X`,
|
||||
() => `X=git; Y='-C ${outsideRepo} reset --hard'; $X $Y`,
|
||||
() =>
|
||||
`eval 'GIT_WORK_TREE=${outsideRepo}'; export GIT_WORK_TREE; git reset --hard`,
|
||||
() => `GIT_WORK_TREE=${outsideRepo}; export $NAME; git reset --hard`,
|
||||
`eval 'GIT_WORK_TREE=${cmdPath(outsideRepo)}'; export GIT_WORK_TREE; git reset --hard`,
|
||||
() =>
|
||||
`GIT_WORK_TREE=${cmdPath(outsideRepo)}; export $NAME; git reset --hard`,
|
||||
])('resolves a relocation through shell variables %#', async (build) => {
|
||||
const guard = createDaemonToolGuard();
|
||||
|
||||
|
|
@ -2402,7 +2426,7 @@ it -C ${outsideRepo} reset --hard`,
|
|||
for (const command of [
|
||||
"env -iS 'git status'",
|
||||
// A `cd` target the guard already knows the value of.
|
||||
`d=${insideNested}; cd $d; git status`,
|
||||
`d=${cmdPath(insideNested)}; cd $d; git status`,
|
||||
// `set +a` turns allexport back off.
|
||||
`set -a; set +a; GIT_WORK_TREE=${outsideRepo}; echo done`,
|
||||
// Definitions used inside the boundary stay allowed.
|
||||
|
|
@ -2431,9 +2455,10 @@ it -C ${outsideRepo} reset --hard`,
|
|||
() => `echo "$'$(GIT_DIR=${plainOutside}/.git git reset --hard HEAD~1)'"`,
|
||||
// The export attribute sticks to the name, so a LATER assignment to it
|
||||
// reaches the git subprocess.
|
||||
() => `export GIT_DIR; GIT_DIR=${plainOutside}; git reset --hard`,
|
||||
() =>
|
||||
`export GIT_WORK_TREE; GIT_WORK_TREE=${plainOutside}; git reset --hard`,
|
||||
`export GIT_DIR; GIT_DIR=${cmdPath(plainOutside)}; git reset --hard`,
|
||||
() =>
|
||||
`export GIT_WORK_TREE; GIT_WORK_TREE=${cmdPath(plainOutside)}; git reset --hard`,
|
||||
// Both sides of a pipe run in subshells: the parent stays outside.
|
||||
() => `cd ${plainOutside}; echo x | cd ${effectiveCwd}; git commit -m x`,
|
||||
// A bare digit before a spaced redirect is a real argv word.
|
||||
|
|
|
|||
|
|
@ -1655,7 +1655,9 @@ describe('Server Config (config.ts)', () => {
|
|||
});
|
||||
|
||||
expect(child.getCwd()).toBe('/tmp/derived');
|
||||
expect(parent.getCwd()).toBe(baseParams.targetDir);
|
||||
// The constructor resolves targetDir, so on win32 the POSIX fixture
|
||||
// spelling comes back drive-qualified — compare the resolved form.
|
||||
expect(parent.getCwd()).toBe(path.resolve(baseParams.targetDir));
|
||||
expect(Object.getPrototypeOf(child)).toBe(parent);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -4,7 +4,6 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import * as os from 'node:os';
|
||||
import type { SimpleGit, SimpleGitFactory, SimpleGitOptions } from 'simple-git';
|
||||
import type { ExtensionInstallMetadata } from '../config/config.js';
|
||||
import type { GitCredential } from './extension-git-credentials.js';
|
||||
|
|
@ -126,7 +125,11 @@ export function createExtensionGitClient(
|
|||
|
||||
const environment: Record<string, string> = {
|
||||
GIT_CONFIG_NOSYSTEM: '1',
|
||||
GIT_CONFIG_GLOBAL: os.devNull,
|
||||
// The literal '/dev/null' on every platform, NOT os.devNull: Git for
|
||||
// Windows special-cases the POSIX spelling in its compat layer, while
|
||||
// os.devNull's win32 value ('\\\\.\\nul') is rejected as
|
||||
// "fatal: unable to access '\\.\nul': Invalid argument".
|
||||
GIT_CONFIG_GLOBAL: '/dev/null',
|
||||
};
|
||||
for (const key of [
|
||||
'PATH',
|
||||
|
|
|
|||
|
|
@ -370,8 +370,22 @@ describe('binding semantics through the real kernel', () => {
|
|||
expect(r.status).toBe('ok');
|
||||
expect(textOf(r.events).trim()).toBe('via-symlink');
|
||||
} finally {
|
||||
fs.rmSync(store, { recursive: true, force: true });
|
||||
fs.rmSync(work, { recursive: true, force: true });
|
||||
// The kernel child's cwd is `work`; on Windows a directory that is any
|
||||
// process's cwd cannot be removed (EBUSY). Dispose first, then retry
|
||||
// the removal across the child's asynchronous teardown.
|
||||
manager.dispose();
|
||||
fs.rmSync(store, {
|
||||
recursive: true,
|
||||
force: true,
|
||||
maxRetries: 10,
|
||||
retryDelay: 200,
|
||||
});
|
||||
fs.rmSync(work, {
|
||||
recursive: true,
|
||||
force: true,
|
||||
maxRetries: 10,
|
||||
retryDelay: 200,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -59,6 +59,13 @@ vi.mock('node:url', async () => {
|
|||
if (process.platform !== 'win32' && /^[a-zA-Z]:\\/.test(filePath)) {
|
||||
return actual.pathToFileURL(filePath, { windows: true });
|
||||
}
|
||||
// The mirror case: fixtures spell workspace paths POSIX-style, and on
|
||||
// win32 the real pathToFileURL would drive-qualify them against the
|
||||
// process cwd (file:///C:/workspace/…). Parse them as POSIX so the
|
||||
// expected URLs read the same on every host.
|
||||
if (process.platform === 'win32' && filePath.startsWith('/')) {
|
||||
return actual.pathToFileURL(filePath, { windows: false });
|
||||
}
|
||||
return actual.pathToFileURL(filePath);
|
||||
},
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue