Skip to content

fix: restrict downloaded file permissions - #107

Merged
jbeckwith-oai merged 1 commit into
mainfrom
codex/restrict-download-file-permissions
Aug 19, 2026
Merged

fix: restrict downloaded file permissions#107
jbeckwith-oai merged 1 commit into
mainfrom
codex/restrict-download-file-permissions

Conversation

@jbeckwith-oai

Copy link
Copy Markdown
Contributor

Summary

  • Create newly downloaded files with owner-only permissions for explicit output paths and server-provided filenames.
  • Preserve existing output-file permissions and overwrite behavior, filename collision handling, temporary-file fallback, and traversal protections.
  • Cover existing-file modes, permissive and restrictive umasks, encoded traversal, symlink collisions, and concurrent downloads with synthetic fixtures.

Verification

  • Focused download tests under umasks 000, 022, and 077.
  • go test -race ./pkg/cmd -run '^(TestWriteBinaryResponse|TestCreateDownloadFile)$' -count=5
  • All six generated download endpoint integration tests.
  • ./scripts/lint, go vet ./..., and formatting checks.
  • Full repository and race suites, excluding one pre-existing macOS FIFO cancellation failure independently reproduced on the unchanged base commit.
  • Windows amd64/arm64 and Linux amd64/arm64 compilation.

Copilot AI lite review requested due to automatic review settings August 19, 2026 18:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens default filesystem permissions for downloaded output artifacts created by the CLI, ensuring newly created files are owner-only readable/writable while preserving existing overwrite behavior and filename-collision handling.

Changes:

  • Create explicit output files (when --output is a path) with 0600 permissions instead of 0644.
  • Create server-suggested download filenames (via Content-Disposition) with 0600 permissions when the file is newly created.
  • Expand download-related tests to assert permissions, validate overwrite-mode preservation, and cover traversal/symlink/collision/concurrency scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/cmd/cmdutil.go Switch new output/download file creation modes from 0644 to 0600 for tighter default permissions.
pkg/cmd/cmdutil_test.go Add/extend tests to verify permission behavior and robustness across overwrite, traversal, symlink, and concurrent download cases.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jbeckwith-oai
jbeckwith-oai marked this pull request as ready for review August 19, 2026 18:39
@jbeckwith-oai
jbeckwith-oai requested a review from a team as a code owner August 19, 2026 18:39

@HAYDEN-OAI HAYDEN-OAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Independent security review of the exact requested commit: no introduced P1/P2 findings. Verified owner-only file creation, preserved overwrite behavior, collision and traversal protections, stdout, failure paths, and concurrent downloads with direct base-versus-HEAD reproductions.

@jbeckwith-oai
jbeckwith-oai added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit 7a3a7d1 Aug 19, 2026
9 checks passed
@jbeckwith-oai
jbeckwith-oai deleted the codex/restrict-download-file-permissions branch August 19, 2026 19:06
@openai-sdks openai-sdks Bot mentioned this pull request Aug 19, 2026
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.

3 participants