backnotprop / backnotprop/plannotator
review: harden the probe-failure path against gitattributes/textconv bypass of core.bigFileThreshold
- 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
Assessment
This issue has not been assessed yet.