fix(pi): harden the Babysit push against a repository the agent controls

Every host-side guard before the push was computed by a git invocation the
agent could influence from inside `.git/`, which is never committed and so is
covered by neither the config digest nor any content check.

- Diff measurement ignored `.git/info/attributes`. One line saying `* -diff`
  reduced a 500 KB change to `Binary files differ`: verified locally as 119
  bytes reported against a real 500,147, so the 200 KB cumulative bound never
  fired, and the `diff` block output — the only record of what an unattended,
  review-comment-steered agent pushed — recorded the same nothing. Both diffs
  now run `--text --no-ext-diff --no-textconv` with an empty
  `core.attributesFile`.
- `rev-list --count` and `merge-base --is-ancestor` answered about replaced
  history. A `refs/replace/*` mapping makes a five-commit chain report one,
  while the pushed SHA stays real. `GIT_NO_REPLACE_OBJECTS` is now set for
  both the prepare and push commands.
- The push asserted HEAD's *shape* — right branch, one commit past the pin, a
  descendant — every part of which survives `commit --amend` to a different
  tree. So the bounds, the quoted-path refusal, the `.github/` refusal and the
  reported diff could all describe a commit other than the one pushed. The
  validated SHA is now passed in and the refspec names it.
- The digest comparison resolved `cat`, `sha256sum` and `cut` through `$PATH`
  in the same script that invokes git absolutely for exactly that reason, and
  was handed the expected digest in its own environment — so a shim could
  print it. The utilities are now absolute.
- The clone left the token in `.git/config` when the pinned-SHA assertion
  failed, because `set -e` aborted before `remote set-url`. Reordered.
- `changedFiles` reached the block output unscrubbed on the Babysit path,
  where Create PR already scrubs it. Names are agent-chosen.

These reduce exposure; they do not bound it. E2B runs each command as
`bash -l`, so a login-shell profile the agent writes executes ahead of the
next host command with the token in its environment. Removing the token from
the sandbox entirely means pushing host-side through the Git Data API, which
the ≤50-file / ≤200 KB bound already makes practical.

Author: icecrasher321

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

@@ -516,14 +516,43 @@ describe('runBabysitPiWithOptions', () => {
     expect(pushCall?.command.indexOf('CURRENT_DIGEST=')).toBeLessThan(
       pushCall?.command.indexOf('/usr/bin/git') ?? 0
     )
-    expect(pushCall?.command).toContain('HEAD:refs/heads/$HEAD_REF')
+    // The refspec names the validated SHA, not HEAD. Asserting only HEAD's shape
+    // left every host-side check describing a commit other than the pushed one,
+    // because `commit --amend` preserves branch, count, and ancestry.
+    expect(pushCall?.command).toContain('"$NEW_SHA:refs/heads/$HEAD_REF"')
+    expect(pushCall?.command).toContain('test "$(/usr/bin/git rev-parse HEAD)" = "$NEW_SHA"')
     expect(pushCall?.envs).toMatchObject({
       GITHUB_TOKEN: 'github-secret',
       ORIGINAL_GIT_CONFIG_DIGEST: 'digest-1',
       PINNED_SHA: OLD_SHA,
+      NEW_SHA,
+      GIT_NO_REPLACE_OBJECTS: '1',
     })
   })
 
+  // A one-line `.git/info/attributes` saying `* -diff` made a 500 KB change report
+  // ~119 bytes to the cumulative bound and wrote "Binary files differ" into the diff
+  // the user reviews. `.git/` is never committed, so the config digest does not see it.
+  it('measures diffs immune to repository-supplied attributes', async () => {
+    mockFetchSnapshot.mockResolvedValue(snapshot)
+    mockFetchThreads.mockResolvedValue({
+      actionable: [trustedThread],
+      skipped: [],
+      totalUnresolved: 1,
+      latestReview: null,
+    })
+    mockFetchChecks.mockResolvedValue(greenChecks)
+    const { runner, runCalls } = makeRunner({})
+    mockWithPiSandbox.mockImplementation(async (callback) => callback(runner))
+
+    await runBabysitPiWithOptions(params(), { onEvent: vi.fn() })
+
+    const prepare = runCalls.find(({ command }) => command.includes('__CUMULATIVE_CHANGED__'))
+    expect(prepare?.command).toContain('core.attributesFile=/dev/null')
+    expect(prepare?.command).toContain('--text --no-ext-diff --no-textconv')
+    expect(prepare?.envs).toMatchObject({ GIT_NO_REPLACE_OBJECTS: '1' })
+  })
+
   it('aggregates pushed rounds while enforcing cumulative markers', async () => {
     mockFetchSnapshot
       .mockResolvedValueOnce(snapshot)

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

@@ -123,25 +123,36 @@ const BABYSIT_GUIDANCE =
   'shape: {"threads":[{"threadId":"…","classification":"fixed|false_positive|already_addressed",' +
   '"reply":"…"}],"summary":"optional"}. Include only fetched thread IDs.'
 
+// `remote set-url` runs before the pinned-SHA assertion, not after: `set -e` aborts
+// the script at a failed assertion, and the clone URL carries the token, so the
+// original order left `https://x-access-token:<token>@…` in `.git/config` on disk
+// whenever the head branch moved between Create PR and this clone.
 const BABYSIT_CLONE_SCRIPT = `set -e
 git check-ref-format "refs/heads/$HEAD_REF" >/dev/null
 rm -rf ${REPO_DIR}
 git clone --no-tags --single-branch --branch "$HEAD_REF" "https://x-access-token:$GITHUB_TOKEN@github.com/$REPO_OWNER/$REPO_NAME.git" ${REPO_DIR}
 cd ${REPO_DIR}
-test "$(git rev-parse HEAD)" = "$PINNED_SHA"
 git remote set-url origin "https://github.com/$REPO_OWNER/$REPO_NAME.git"
+test "$(git rev-parse HEAD)" = "$PINNED_SHA"
 ${GIT_CONFIG_DIGEST_LINE}`
 
 /**
  * Stages, commits, and bounds one round's work before the separately
  * authenticated push.
  *
- * The name-listing diffs run with `core.quotePath=false`. Git's default quotes
- * any path containing a non-ASCII or special byte — `.github/workflows/évil.yml`
- * is emitted as `".github/workflows/\303\251vil.yml"`, leading double quote
- * included — which would make the host-side `.github/` refusal in
- * {@link finalizeRound} compare against a quoted string and miss. The byte-count
- * diff is deliberately left alone: it measures content, not names.
+ * Every `git diff` here is neutralized against repository-supplied configuration,
+ * because the agent owns the checkout and `.git/` is not part of the commit:
+ *
+ * - `core.quotePath=false` stops the non-ASCII escaping that made a `.github/`
+ *   path arrive as `".github/workflows/\303\251vil.yml"` and miss the host-side
+ *   prefix test. Paths Git still quotes are refused outright in
+ *   {@link finalizeRound}.
+ * - `--text --no-ext-diff --no-textconv` and an empty `core.attributesFile` stop
+ *   a one-line `.git/info/attributes` saying `* -diff` from reducing a 500 KB
+ *   change to `Binary files differ` — which reported ~119 bytes to the cumulative
+ *   byte bound and wrote the same nothing into the diff the user reviews.
+ * - `GIT_NO_REPLACE_OBJECTS` is set in the env so `rev-list --count` counts real
+ *   commits: a `refs/replace/*` mapping makes a five-commit chain report one.
  */
 const BABYSIT_PREPARE_SCRIPT = `set -e
 cd ${REPO_DIR}
@@ -157,24 +168,39 @@ else
   test "$(git rev-list --count "$ROUND_BASE_SHA"..HEAD)" = "1"
   test "$(git symbolic-ref --short HEAD)" = "$HEAD_REF"
   test "$(git rev-parse "refs/heads/$HEAD_REF")" = "$(git rev-parse HEAD)"
-  git -c core.quotePath=false diff --name-only "$INITIAL_HEAD_SHA" HEAD | sed "s/^/__CUMULATIVE_CHANGED__=/"
-  git diff "$INITIAL_HEAD_SHA" HEAD | wc -c | tr -d ' ' | sed "s/^/__CUMULATIVE_DIFF_BYTES__=/"
-  git -c core.quotePath=false diff --name-only "$ROUND_BASE_SHA" HEAD | sed "s/^/__CHANGED__=/"
-  git diff "$ROUND_BASE_SHA" HEAD > ${DIFF_PATH}
+  git -c core.quotePath=false -c core.attributesFile=/dev/null diff --name-only --no-ext-diff "$INITIAL_HEAD_SHA" HEAD | sed "s/^/__CUMULATIVE_CHANGED__=/"
+  git -c core.attributesFile=/dev/null diff --text --no-ext-diff --no-textconv "$INITIAL_HEAD_SHA" HEAD | wc -c | tr -d ' ' | sed "s/^/__CUMULATIVE_DIFF_BYTES__=/"
+  git -c core.quotePath=false -c core.attributesFile=/dev/null diff --name-only --no-ext-diff "$ROUND_BASE_SHA" HEAD | sed "s/^/__CHANGED__=/"
+  git -c core.attributesFile=/dev/null diff --text --no-ext-diff --no-textconv "$ROUND_BASE_SHA" HEAD > ${DIFF_PATH}
   git rev-parse HEAD | sed "s/^/__NEW_SHA__=/"
   test -z "$(git status --porcelain)"
   echo "__NEEDS_PUSH__=1"
 fi`
 
+/**
+ * The only token-bearing command after the clone.
+ *
+ * `NEW_SHA` pins the push to the exact commit the host validated. Without it the
+ * script asserted only HEAD's *shape* — right branch, one commit past the pin, a
+ * descendant — all of which still hold after a `commit --amend` to a different
+ * tree, so every host-side check (bounds, quoted paths, `.github/`, the reported
+ * diff) described a commit other than the one pushed. The refspec names the SHA
+ * for the same reason.
+ *
+ * The shell utilities are absolute for the reason `git` already was: `$PATH` is
+ * writable by an agent that has had root in this sandbox for previous rounds, and
+ * a shim named `sha256sum` would otherwise be handed the digest it must produce.
+ */
 const BABYSIT_PUSH_SCRIPT = `set -e
 cd ${REPO_DIR}
-CURRENT_DIGEST=$(cat .git/config .git/config.worktree 2>/dev/null | sha256sum | cut -d' ' -f1)
+CURRENT_DIGEST=$(/bin/cat .git/config .git/config.worktree 2>/dev/null | /usr/bin/sha256sum | /usr/bin/cut -d' ' -f1)
 test "$CURRENT_DIGEST" = "$ORIGINAL_GIT_CONFIG_DIGEST"
+test "$(/usr/bin/git rev-parse HEAD)" = "$NEW_SHA"
 test "$(/usr/bin/git symbolic-ref --short HEAD)" = "$HEAD_REF"
-test "$(/usr/bin/git rev-parse "refs/heads/$HEAD_REF")" = "$(/usr/bin/git rev-parse HEAD)"
-test "$(/usr/bin/git rev-list --count "$PINNED_SHA"..HEAD)" = "1"
-/usr/bin/git merge-base --is-ancestor "$PINNED_SHA" HEAD
-/usr/bin/git -c core.hooksPath=/dev/null -c credential.helper= -c core.fsmonitor= push "https://x-access-token:$GITHUB_TOKEN@github.com/$REPO_OWNER/$REPO_NAME.git" "HEAD:refs/heads/$HEAD_REF"
+test "$(/usr/bin/git rev-parse "refs/heads/$HEAD_REF")" = "$NEW_SHA"
+test "$(/usr/bin/git rev-list --count "$PINNED_SHA".."$NEW_SHA")" = "1"
+/usr/bin/git merge-base --is-ancestor "$PINNED_SHA" "$NEW_SHA"
+/usr/bin/git -c core.hooksPath=/dev/null -c credential.helper= -c core.fsmonitor= push "https://x-access-token:$GITHUB_TOKEN@github.com/$REPO_OWNER/$REPO_NAME.git" "$NEW_SHA:refs/heads/$HEAD_REF"
 echo "__PUSHED__=1"`
 
 interface BabysitBackendOptions {
@@ -524,6 +550,7 @@ async function finalizeRound(
         HEAD_REF: snapshot.headRef,
         INITIAL_HEAD_SHA: initialHeadSha,
         ROUND_BASE_SHA: roundBaseSha,
+        GIT_NO_REPLACE_OBJECTS: '1',
       },
       timeoutMs: FINALIZE_TIMEOUT_MS,
     }),
@@ -589,7 +616,9 @@ async function finalizeRound(
         REPO_NAME: params.repo,
         HEAD_REF: snapshot.headRef,
         PINNED_SHA: snapshot.headSha,
+        NEW_SHA: newSha,
         ORIGINAL_GIT_CONFIG_DIGEST: gitConfigDigest,
+        GIT_NO_REPLACE_OBJECTS: '1',
       },
       timeoutMs: FINALIZE_TIMEOUT_MS,
     }),
@@ -994,8 +1023,15 @@ export async function runBabysitPiWithOptions(
           roundBaseSha = finalized.newSha
           progress.commitsPushed += 1
           githubWriteOccurred = true
+          // Scrubbed here rather than in `finalizeRound`, which needs the literal
+          // paths for its `.github/` and quoted-path refusals. File names are
+          // agent-chosen, so a file named after a key would otherwise reach the
+          // block output verbatim — Create PR already scrubs its equivalent.
           progress.changedFiles = [
-            ...new Set([...progress.changedFiles, ...finalized.changedFiles]),
+            ...new Set([
+              ...progress.changedFiles,
+              ...finalized.changedFiles.map((file) => scrubPiSecrets(file, secrets)),
+            ]),
           ]
           progress.diff = capDiff([progress.diff, finalized.diff].filter(Boolean).join('\n'))
           lastKnownChecksGreen = ![...initialRequirements.values()].some((required) => required)

apps/sim/tools/github/job_logs.test.ts

@@ -16,7 +16,7 @@ function logResponse(body: string): Response {
   return new Response(body, { headers: { 'Content-Type': 'text/plain' } })
 }
 
-/** A storage host that honoured the suffix range: 206 plus the full size. */
+/** A storage host that honoured the suffix range: 206 plus the served window. */
 function partialLogResponse(body: string, totalBytes: number): Response {
   return new Response(body, {
     status: 206,
@@ -100,6 +100,25 @@ describe('github_job_logs', () => {
     })
   })
 
+  // A suffix range asking for more bytes than the log holds is satisfied with the
+  // WHOLE log, still as a 206 — `Content-Range` starts at 0. Trimming the first line
+  // there would delete a real line, and this is the common case for a job that
+  // failed fast and logged little.
+  it('keeps the first line when a 206 served the whole log', async () => {
+    const log = 'first line\nsecond line\n'
+
+    const result = await jobLogsTool.transformResponse!(
+      partialLogResponse(log, Buffer.byteLength(log)),
+      { ...BASE_PARAMS, maxCharacters: 20_000 }
+    )
+
+    expect(result.output).toEqual({
+      logs: log,
+      truncated: false,
+      totalBytes: Buffer.byteLength(log),
+    })
+  })
+
   // A host that ignores the range answers 200 with the whole body, so the local
   // slice has to remain the fallback rather than an assumption about partiality.
   it('falls back to the local slice when the range is ignored', async () => {

apps/sim/tools/github/job_logs.ts

@@ -28,44 +28,58 @@ function jobLogsPath(owner: string, repo: string, jobId: number): string {
   return `${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/actions/jobs/${jobId}/logs`
 }
 
+/** Byte offsets from a `Content-Range: bytes <start>-<end>/<total>` header. */
+interface ContentRange {
+  start: number
+  total: number | null
+}
+
 /**
- * Total size from a `Content-Range: bytes <start>-<end>/<total>` header.
+ * Parses the served byte window.
  *
- * `null` for an unsatisfied-range form, an unknown total, an unparsable value, or
- * an absent header — all of which mean the full size is simply unknown here.
+ * `null` for an unsatisfied-range form, an unparsable value, or an absent header.
+ * The `start` matters as much as the total: a suffix range asking for more bytes
+ * than the log contains is satisfied with the *whole* representation, still as a
+ * 206, and only `start === 0` distinguishes that from a window that genuinely cut
+ * into the middle of the log.
  */
-function parseContentRangeTotal(header: string | null): number | null {
-  const total = header?.match(/^bytes\s+\d+-\d+\/(\d+)$/)?.[1]
-  if (!total) return null
-  const parsed = Number(total)
-  return Number.isSafeInteger(parsed) && parsed >= 0 ? parsed : null
+function parseContentRange(header: string | null): ContentRange | null {
+  const match = header?.match(/^bytes\s+(\d+)-\d+\/(\d+|\*)$/)
+  if (!match) return null
+  const start = Number(match[1])
+  if (!Number.isSafeInteger(start) || start < 0) return null
+  const total = match[2] === '*' ? null : Number(match[2])
+  return {
+    start,
+    total: total !== null && Number.isSafeInteger(total) && total >= 0 ? total : null,
+  }
 }
 
 /**
  * The tail is what matters: a failing job reports its error at the end.
  *
- * A ranged response already *is* the tail, so it is only trimmed at the first
- * line break — the byte window almost always cuts mid-line, and it can also split
- * a multi-byte character into a replacement char. A full response is sliced
- * locally instead, which is the path taken whenever the storage host ignores the
- * range and answers 200.
+ * A window that starts partway into the log is trimmed at its first line break,
+ * because the byte boundary almost always lands mid-line and can split a
+ * multi-byte character. A window starting at zero is the whole log — the storage
+ * host satisfied a suffix range larger than the content — so it is treated
+ * exactly like an unranged body, which is the common case for a job that failed
+ * fast and logged little.
  */
 function logTail(
   text: string,
   maxCharacters: number,
-  partial: boolean,
-  totalBytes: number | null
+  range: ContentRange | null
 ): { logs: string; truncated: boolean; totalBytes: number | null } {
-  if (!partial) {
+  if (!range || range.start === 0) {
     return {
       logs: text.slice(-maxCharacters),
       truncated: text.length > maxCharacters,
-      totalBytes: totalBytes ?? Buffer.byteLength(text),
+      totalBytes: range?.total ?? Buffer.byteLength(text),
     }
   }
   const firstBreak = text.indexOf('\n')
   const trimmed = firstBreak === -1 ? text : text.slice(firstBreak + 1)
-  return { logs: trimmed.slice(-maxCharacters), truncated: true, totalBytes }
+  return { logs: trimmed.slice(-maxCharacters), truncated: true, totalBytes: range.total }
 }
 
 export const jobLogsTool: ToolConfig<JobLogsParams, JobLogsResponse> = {
@@ -133,13 +147,20 @@ export const jobLogsTool: ToolConfig<JobLogsParams, JobLogsResponse> = {
     stripAuthOnRedirect: true,
   },
 
+  /**
+   * A suffix range against a zero-length log is unsatisfiable, so such a job
+   * surfaces as a 416 tool error rather than as empty `logs`. Not special-cased
+   * here because the executor rejects a non-2xx before `transformResponse` runs,
+   * and an Actions job log is never truly empty — the runner writes its own
+   * setup lines before any step does.
+   */
   transformResponse: async (response, params) => {
     const maxCharacters = resolveMaxCharacters(params?.maxCharacters)
-    const partial = response.status === 206
-    const totalBytes = parseContentRangeTotal(response.headers.get('content-range'))
+    const range =
+      response.status === 206 ? parseContentRange(response.headers.get('content-range')) : null
     return {
       success: true,
-      output: logTail(await response.text(), maxCharacters, partial, totalBytes),
+      output: logTail(await response.text(), maxCharacters, range),
     }
   },