Skip to content

fix(git-read): reject NUL bytes in paths - #613

Merged
tt-a1i merged 2 commits into
openpi-dev:mainfrom
outlier27-cell:audit/git-path-nul
Sep 24, 2026
Merged

tt-a1i merged 2 commits into
openpi-dev:mainfrom
outlier27-cell:audit/git-path-nul

Conversation

@outlier27-cell

Copy link
Copy Markdown
Contributor

ProblemnnCloses #612. Git path validation accepted NUL bytes, then Node rejected the argv at spawn time with ERR_INVALID_ARG_VALUE instead of OpenPI returning InvalidPathError.nn## ValuennReject unsupported paths at the tool boundary with a stable, actionable error.nn## ApproachnnReject NUL in the shared repository-path validator and cover show, diff, and log argv builders.nn## Validationnn- Red: latest main accepted src\0secret.ts; Node rejected it only at spawn.n- Focused args tests: 4/4 passed.n- Git-read suite: 27/27 passed.n- bun run check: passed.n- bun run test: changed Git-read tests passed in the full run; it then reproduced the known unrelated Windows real-Pi post-edit timeout cluster, so no full-suite pass is claimed.nn## Impactnn- User-visible: NUL paths return existing InvalidPathError.n- Model-visible tools, runtime/lifecycle, persistence: none.n- Compatibility: NUL cannot be passed to Node process argv or name a normal repository path.

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the NUL path guard and argv-builder coverage at 9b9ad43. No blocking findings; exact-head CI is green.

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked the updated head f88163d against current main. The PR still changes only the NUL path guard and its argv tests; no blocking findings.

@tt-a1i
tt-a1i merged commit b938a39 into openpi-dev:main Sep 24, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants