Skip to content

Commit a4aff13

Browse files
committed
fix(pi): refuse any Git-quoted path before the Babysit push
`core.quotePath=false` stopped Git escaping non-ASCII bytes, but it closed one instance rather than the class. Git still quotes any path it cannot state on a single line — one containing a newline, a double quote, a backslash, or a tab — and such a path arrives with a leading `"`, so the `.github/` prefix test does not match it and the refusal is bypassed exactly as before. Refuse any quoted path outright. Unescaping instead would put a second implementation of Git's quoting rules on the security-relevant side of the push, and these characters have no legitimate place in a source path.
1 parent fa8a56a commit a4aff13

2 files changed

Lines changed: 51 additions & 0 deletions

File tree

apps/sim/executor/handlers/pi/babysit-backend.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,34 @@ describe('runBabysitPiWithOptions', () => {
652652
expect(prepare?.command).toContain('core.quotePath=false')
653653
})
654654

655+
// `core.quotePath=false` stops the non-ASCII escaping but Git still quotes a path
656+
// containing a newline, quote, backslash, or tab — which arrives with a leading `"`
657+
// and so slips past a `.github/` prefix test. Any quoted path is refused outright.
658+
it.each([
659+
['newline', '".github/workflows/new\\nline.yml"'],
660+
['double quote', '".github/quo\\"te.yml"'],
661+
['backslash', '".github/back\\\\slash.yml"'],
662+
['tab', '".github/tab\\tx.yml"'],
663+
])('refuses a %s path that Git could not report literally', async (_label, quotedPath) => {
664+
mockFetchSnapshot.mockResolvedValue(snapshot)
665+
mockFetchThreads.mockResolvedValue({
666+
actionable: [trustedThread],
667+
skipped: [],
668+
totalUnresolved: 1,
669+
latestReview: null,
670+
})
671+
mockFetchChecks.mockResolvedValue(greenChecks)
672+
const { runner, runCalls } = makeRunner({
673+
prepareStdout: `__CUMULATIVE_CHANGED__=${quotedPath}\n__CUMULATIVE_DIFF_BYTES__=20\n__CHANGED__=${quotedPath}\n__NEW_SHA__=${NEW_SHA}\n__NEEDS_PUSH__=1\n`,
674+
})
675+
mockWithPiSandbox.mockImplementation(async (callback) => callback(runner))
676+
677+
const result = await runBabysitPiWithOptions(params(), { onEvent: vi.fn() })
678+
679+
expect(result).toMatchObject({ stopReason: 'refused_content', commitsPushed: 0 })
680+
expect(runCalls.some(({ command }) => command.includes('CURRENT_DIGEST='))).toBe(false)
681+
})
682+
655683
it('reports a hardened push rejection without losing partial counters', async () => {
656684
mockFetchSnapshot.mockResolvedValue(snapshot)
657685
mockFetchThreads.mockResolvedValue({

apps/sim/executor/handlers/pi/babysit-backend.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,23 @@ function trimUntrustedJson<T>(
242242
return { json: untrustedJson([]), omitted: payload.length }
243243
}
244244

245+
/**
246+
* Whether Git reported a path in its quoted form rather than literally.
247+
*
248+
* `core.quotePath=false` stops Git escaping non-ASCII bytes, but it still quotes
249+
* any path it could not otherwise fit on one line — one containing a newline,
250+
* a double quote, a backslash, or a tab. Such a path arrives with a leading `"`,
251+
* so a prefix test like the `.github/` refusal below silently fails to match it.
252+
*
253+
* Refusing outright rather than unescaping: these characters have no legitimate
254+
* place in a source path, and a decoder here would be a second parser of Git's
255+
* quoting rules sitting on the security-relevant side of the push. A path Git
256+
* declined to state plainly is one this code declines to push.
257+
*/
258+
function isQuotedGitPath(path: string): boolean {
259+
return path.startsWith('"')
260+
}
261+
245262
/**
246263
* Bounds a diff to the same ceiling Create PR applies to its own.
247264
*
@@ -547,6 +564,12 @@ async function finalizeRound(
547564
'Babysit cumulative change bounds were exceeded'
548565
)
549566
}
567+
if (cumulativeChangedFiles.some(isQuotedGitPath)) {
568+
throw new BabysitGitHubError(
569+
'refused_content',
570+
'Babysit refuses to push a path Git could not report literally'
571+
)
572+
}
550573
if (cumulativeChangedFiles.some((file) => file === '.github' || file.startsWith('.github/'))) {
551574
throw new BabysitGitHubError(
552575
'refused_content',

0 commit comments

Comments
 (0)