Skip to content

fix: harden proxy config and CLI input validation#98

Open
Alanperry1 wants to merge 3 commits intoopenai:mainfrom
Alanperry1:main
Open

fix: harden proxy config and CLI input validation#98
Alanperry1 wants to merge 3 commits intoopenai:mainfrom
Alanperry1:main

Conversation

@Alanperry1
Copy link
Copy Markdown

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Alanperry1
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@Alanperry1
Copy link
Copy Markdown
Author

recheck

github-actions Bot added a commit that referenced this pull request May 7, 2026
Copilot AI review requested due to automatic review settings May 7, 2026 06:04
@Alanperry1
Copy link
Copy Markdown
Author

Alanperry1 commented May 7, 2026

Summary

This PR hardens codex-action around config writes, temp file handling, and CLI validation.

It fixes a repeated-run issue where proxy config could accumulate duplicate managed entries, resolves an unprivileged-user ownership mismatch when writing inline output schemas, and tightens validation for proxy ports and codex-args so failures happen earlier and with clearer errors.

What Changed

  • made proxy config writes idempotent using verbatim marker-block removal — surrounding user content is no longer trimmed or reformatted
  • strip any top-level model_provider value (not just the managed one) before prepending the managed block to prevent duplicate TOML keys
  • fixed inline output-schema temp file creation and cleanup for unprivileged-user
  • added shared port validation; non-integer or out-of-range values are rejected in both CLI parsing and server-info reads
  • wrapped malformed codex-args JSON with clearer user-facing errors; enforce array-of-strings input
  • handle leading whitespace before [ in codex-args so inputs like ' ["--flag"]' are parsed as JSON instead of being shell-split
  • bumped setup-node to v6.4.0 (Update the internal setup-node dependency to a Node 24-compatible version #84)
  • removed runner.environment == 'github-hosted' bwrap guard so the sandbox applies on self-hosted runners too (v1.5 sandbox regression: bwrap loopback fails on GitHub Actions runner #88)
  • added ENOENT hint in readServerInfo when the proxy socket file is missing (Improve missing API key error: avoid ENOENT on missing server info JSON #70)
  • cleaned up test temp directories in try/finally; added regression tests for repeated proxy config writes, duplicate model_provider keys, invalid ports, and invalid codex-args

Why

These paths were a little too easy to break:

  • repeated action runs could leave duplicate proxy config entries or reformat unrelated parts of the user's config
  • a user-defined model_provider in their config combined with our managed block would produce invalid TOML with duplicate keys
  • unprivileged-user runs could fail on schema temp files because of ownership mismatches
  • invalid port values could slip through and fail later in less obvious ways
  • malformed codex-args surfaced raw JSON parse errors instead of actionable feedback
  • codex-args JSON with leading whitespace was silently routed to the shell splitter

Testing

  • npm run build
  • node --test test/*.test.mjs (13/13 pass)
  • npm run check

Copy link
Copy Markdown

Copilot AI left a comment

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 hardens the CLI and proxy config management by introducing strict port validation, improving --extra-args JSON handling, and making write-proxy-config idempotently replace previously managed config blocks rather than duplicating entries.

Changes:

  • Add a shared port validation module and apply it to CLI parsing and server-info parsing.
  • Make write-proxy-config replace prior codex-action-managed proxy config on repeated runs.
  • Improve CLI validation/error reporting for malformed --extra-args JSON and add regression tests.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/writeProxyConfig.test.mjs Adds a regression test ensuring repeated write-proxy-config runs don’t duplicate managed config.
test/cliValidation.test.mjs Adds CLI validation tests for invalid ports and malformed/non-string --extra-args JSON.
src/writeProxyConfig.ts Implements managed-block merge/strip logic for proxy config generation.
src/runCodexExec.ts Fixes temp output-schema handling when running as an unprivileged user (sudo-aware create/write/cleanup).
src/readServerInfo.ts Validates the server info port value using shared port validation.
src/ports.ts Introduces reusable port parsing/validation helpers.
src/main.ts Switches port parsing to the new validator and hardens --extra-args JSON parsing errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/writeProxyConfig.ts
Comment thread src/writeProxyConfig.ts Outdated
Comment thread src/main.ts
Comment thread test/writeProxyConfig.test.mjs
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