backnotprop / backnotprop/plannotator

review: harden the probe-failure path against gitattributes/textconv bypass of core.bigFileThreshold

Open
#1,206 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
8.7k
Forks
649
PR merge metrics
PR metrics pending

Description

**TLDR:** #1205 made large-diff memory bounding survive a failed object-size probe by injecting `core.bigFileThreshold` into the patch-rendering git calls. Adversarial review confirmed one narrow gap: git applies that threshold only when binaryness is undetermined, so a repo whose own `.gitattributes` marks a >5MB changed file as `diff` (or gives it a textconv driver) renders the full blob on the probe-failure path. Reaching it requires the conjunction of a failed/timed-out probe and such an attributed file, and with a working probe (the norm) the JS-side exclusion still applies, so this is a hardening follow-up rather than a regression.

**Suggested fixes** (either or both):
- Append `--no-textconv` (and consider `-a` suppression via attr overrides) to rendered diff args when the probe returned null.
- Add a hard stdout cap in both runtimes' git runners (`packages/server/git.ts` and `apps/pi-extension/server/vcs.ts`). This is the only true invariant: no single git invocation should be able to buffer unbounded output regardless of cause.

**Also noted during review:** on git 2.50.1 the threshold empirically DID bound the dirty worktree side in every shape tried, contradicting the rationale text in #1205's description. Harmless, since the stat door is kept regardless (defense in depth for gits where it does not hold), but worth knowing when reading that PR.

Found by adversarial review of #1205 before merge. AI-assisted.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.