Skip to content

fix: #3273 validate git repo subpaths#3276

Merged
seratch merged 2 commits intoopenai:mainfrom
Aphroq:fix/git-repo-subpath-validation
May 9, 2026
Merged

fix: #3273 validate git repo subpaths#3276
seratch merged 2 commits intoopenai:mainfrom
Aphroq:fix/git-repo-subpath-validation

Conversation

@Aphroq
Copy link
Copy Markdown
Contributor

@Aphroq Aphroq commented May 8, 2026

Summary

  • Add validation for GitRepo.subpath before copying from the cloned repository.
  • Reject empty, absolute, Windows-style, and parent traversal subpaths with a structured GitSubpathError.
  • Add regression tests covering invalid subpaths and a valid relative subpath.

Test plan

  • uv run pytest tests/sandbox/test_entries.py -k "git_repo"
  • bash .agents/skills/code-change-verification/scripts/run.sh

Issue number

Closes #3273

Checks

  • I've added new tests (if relevant)
  • I've added/updated the relevant documentation
  • I've run make lint and make format
  • I've made sure tests pass

@github-actions github-actions Bot added bug Something isn't working feature:sandboxes labels May 8, 2026
@Aphroq Aphroq changed the title fix: validate git repo subpaths fix: #3273 validate git repo subpaths May 8, 2026
Copy link
Copy Markdown
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

The intent of validating git_repo.subpath makes sense, but this currently validates too late.

With an invalid value like subpath="../outside", the code still performs the git clone first and only raises GitSubpathError afterward. Since this is a local configuration error and can be detected without touching the network or filesystem-heavy clone path, we should fail fast before cloning.

Could we move the subpath validation ahead of the clone step so invalid config is rejected before any repository fetch work starts?

Also, the CI is failing now.

@seratch seratch marked this pull request as draft May 8, 2026 16:40
@Aphroq Aphroq force-pushed the fix/git-repo-subpath-validation branch from 38b3a49 to 4e45c26 Compare May 8, 2026 23:29
@Aphroq
Copy link
Copy Markdown
Contributor Author

Aphroq commented May 8, 2026

Thanks for the review. I moved the subpath validation ahead of the git availability check and clone/fetch path, so invalid local configuration now raises GitSubpathError before any sandbox command is executed.

I also tightened the invalid-subpath regression test to assert that no session exec calls are made, and fixed CI.

@Aphroq Aphroq marked this pull request as ready for review May 8, 2026 23:34
@seratch seratch added this to the 0.17.x milestone May 9, 2026
@seratch seratch merged commit 55eb3de into openai:main May 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working feature:sandboxes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitRepo.subpath should reject parent traversal paths

2 participants