Skip to content

Conversation

@rogeralsing
Copy link
Contributor

Summary

  • add an apply_patch internal command that applies headless diff patches and can optionally ignore whitespace differences
  • register the command by default and cover whitespace handling plus file addition scenarios with new tests

Testing

  • go test ./...

https://chatgpt.com/codex/tasks/task_e_68fdd91ba6448328800cd2be4ef933ac

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +445 to +453
ensureFileState := func(relativePath string, create bool) (*fileState, error) {
cleanedRel := strings.TrimSpace(relativePath)
if cleanedRel == "" {
return nil, errors.New("apply_patch: empty file path in patch")
}
absPath, err := filepath.Abs(filepath.FromSlash(cleanedRel))
if err != nil {
return nil, fmt.Errorf("apply_patch: failed to resolve %s: %w", relativePath, err)
}

Choose a reason for hiding this comment

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

P1 Badge Restrict apply_patch file writes to workspace

ensureFileState resolves whatever path the patch supplies to an absolute path and then proceeds to read/write it without checking that it lies under the agent’s workspace. A patch like *** Update File: ../../etc/passwd will therefore touch files outside the repository. Because this internal command is intended to be a safe alternative to shell access, allowing arbitrary path traversal is a security foot‑gun. Consider rejecting paths that escape the workspace root or resolving relative to a fixed base directory before touching the filesystem.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants