vrr/qwen-code
2
0
Fork 0
mirror of https://github.com/QwenLM/qwen-code.git synced 2026-07-10 01:29:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 12

fix(worktree): lexical sanitizer for CodeQL + missing test mock entry

Browse source 
Two fixes from the third CI round on PR #4381:

1. CodeQL re-fires (round 2 of the same finding).

`--end-of-options` is a git-runtime defense, not a lexical sanitizer
that CodeQL's `js/second-order-command-line-injection` taint tracker
recognises. The alert re-fired against the same call after the
previous fix.

Switch to a CodeQL-recognised sanitizer: validate the numeric
component against `/^[1-9][0-9]*$/` immediately at the sink. The
regex digit-only check is one of the documented sanitizer patterns
the rule looks for, and proves at the analyzer level that the
resulting argv element cannot resemble a flag (`--foo`). The entry
guard at the top of the function still establishes the same fact
at runtime; this layer makes the proof visible to static analysis.
Keep `--end-of-options` as a runtime fallback against any future
regression that loosens the entry guard.

2. `nonInteractiveCli.test.ts` mock was missing the new
   `consumePendingStartupWorktreeNotice` Config method.

Phase D-1 added the method on `Config` and `nonInteractiveCli`
calls it on every prompt to pick up the one-shot startup-worktree
notice. The test file's `mockConfig` literal was not updated, so
all 19 `runNonInteractive` tests threw
`TypeError: config.consumePendingStartupWorktreeNotice is not a
function` on Ubuntu / macOS CI.

Add a stub returning `null` so the helper short-circuits, matching
the equivalent Phase C stub for `getResumedSessionData`.

Local: cli (worktreeStartup + nonInteractiveCli) 60 passed + 1
skipped; core (gitWorktreeService + symlinks + hooks +
enter-worktree) 66 passed.
This commit is contained in:
LaZzyMan 2026-05-21 15:53:51 +08:00
parent 23289d6134
commit 000c9f63e5
2 changed files with 33 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

6
packages/cli/src/nonInteractiveCli.test.ts
View file

@ -211,6 +211,12 @@ describe('runNonInteractive', () => {
// restore worktree context. These tests don't exercise resume, so
// return undefined to short-circuit the helper.
getResumedSessionData: vi.fn().mockReturnValue(undefined),
// Phase D-1: nonInteractiveCli calls this on every prompt to pick
// up the one-shot startup-worktree notice (set by gemini.tsx
// when --worktree was passed). These tests don't exercise the
// --worktree flag, so return null to short-circuit injection
// and let the resume-restore branch run.
consumePendingStartupWorktreeNotice: vi.fn().mockReturnValue(null),
} as unknown as Config;
mockSettings = {

35
packages/core/src/services/gitWorktreeService.ts
View file

@ -1236,22 +1236,41 @@ export class GitWorktreeService {
}
const timeoutMs = options?.timeoutMs ?? 30_000;
// Two-layer defense for the refspec argv element:
//
// 1. Regex digit-only validation at the call site — CodeQL's
// `js/second-order-command-line-injection` rule recognises
// `/^[1-9][0-9]*$/.test(x)` as a lexical sanitizer, which proves
// `prNumber` cannot resemble a `--upload-pack=…` flag. The
// entry guard above already establishes this at runtime, but
// CodeQL's interprocedural taint tracker doesn't see through
// that guard; the regex check IS the pattern its sanitizer
// library recognises.
// 2. `--end-of-options` as a git-runtime marker. Even though
// layer 1 makes a flag-shaped refspec impossible, the marker
// tells git definitively that every subsequent argv element
// is positional — defense-in-depth against a future
// regression that loosens the entry guard.
const prNumberStr = String(prNumber);
if (!/^[1-9][0-9]*$/.test(prNumberStr)) {
// Unreachable given the entry guard; here to make the
// lexical sanitizer visible to static analyzers.
return {
success: false,
error: `Invalid PR number: ${prNumber}.`,
};
}
const refspec = `pull/${prNumberStr}/head`;
try {
// Force English git stderr so the error-taxonomy regexes below
// match. Without this, users with non-English locales fall
// through to the generic "PR may not exist" branch even for
// well-known cases like missing-origin. The git binary itself is
// unaffected by LANG/LC_ALL beyond message strings.
//
// `--end-of-options` defends against second-order command injection
// (git's own `--upload-pack` flag) flagged by CodeQL — even though
// `prNumber` is already constrained to a safe positive integer by
// the entry guard above, the marker tells git definitively that
// every subsequent argv element is a positional, not a flag, and
// satisfies the analyzer without runtime cost.
await execFileAsync(
'git',
['fetch', '--end-of-options', 'origin', `pull/${prNumber}/head`],
['fetch', '--end-of-options', 'origin', refspec],
{
cwd: this.sourceRepoPath,
timeout: timeoutMs,