Skip to content

Use --end-of-options before user-controlled refs in git invocations - #290

Open
lohengrin332 wants to merge 1 commit into
openai:mainfrom
lohengrin332:harden-git-end-of-options
Open

Use --end-of-options before user-controlled refs in git invocations#290
lohengrin332 wants to merge 1 commit into
openai:mainfrom
lohengrin332:harden-git-end-of-options

Conversation

@lohengrin332

@lohengrin332 lohengrin332 commented May 2, 2026

Copy link
Copy Markdown

This is a defense-in-depth hardening PR, not a CVE. There is no remotely exploitable bug here in normal use of the plugin.

The behavior being changed: git.mjs passes the user-supplied --base <ref> value (and values derived from it) into git argv positions without --end-of-options. None of the affected git subcommands have known option-injection paths to arbitrary command execution, but the boundary is easy to harden.

Why it's worth tightening: under prompt-injection-adjacent threat models, a hostile --base "--output=/tmp/x" argument reaches git as a positional. The set of dangerous git options the current subcommands accept may grow over time (or may already include something not yet audited). --end-of-options is the documented git idiom for "treat the next argument as a value regardless of leading dashes" and has been supported since 2.24 (November 2019).

A note on -- vs --end-of-options: -- is a path-spec separator in git log <range> and git diff <range>, which would silently change the command's semantics. --end-of-options is the correct construct.

Changes

  • lib/git.mjs: insert --end-of-options before user-input positionals in merge-base, show-ref, and downstream diff/log commands that take ranges derived from user input. Also adds a _gitCheckedImpl injection point to buildBranchComparison (threaded from collectReviewContext) to support structural testing.
  • tests/git.test.mjs: three new tests:
  • Behavioral (3a): hostile --base value of the form --output=<sentinel> errors out as a bad ref and fails to create the sentinel file.
  • Behavioral (3c): refs with embedded dashes (feature/has-dashes-in-name) still work, guarding against any future regex-based "is this a flag?" check.
  • Structural (3d): spy on the injected git runner and assert --end-of-options appears before baseRef in the merge-base argv.

Minimum git version: 2.24 (Nov 2019), which introduced --end-of-options. CI runs ubuntu-latest so this is well below the floor in practice. Happy to add an explicit note in package.json engines or the README if you'd like.

🤖 Generated with Claude Code

@lohengrin332 @claude
Pass --end-of-options before user-controlled refs in git commands
7798180 
User-supplied --base values flow through resolveReviewTarget and
buildBranchComparison into git invocations as positional arguments.
A flag-shaped value like `--output=/tmp/x` could in principle be
parsed by git as an option rather than a ref. The current set of
git subcommands the plugin uses (merge-base, show-ref, diff, log)
do not have known option-injection paths to arbitrary command
execution, but defending the input boundary is cheaper than
relying on that staying true.

Insert --end-of-options before any user-input-derived positional.
This is the documented git idiom (since 2.24, Nov 2019) for "treat
the next argument as a value, not a flag." Note: -- is the wrong
construct here because in `git log <range>` and `git diff <range>`,
-- is a path-spec separator that would silently change semantics.

Tested with a hostile --base value of the form --output=<sentinel>;
the call now errors as a bad ref instead of either succeeding or
creating the sentinel file.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@lohengrin332
lohengrin332 requested a review from a team May 2, 2026 21:03
fireblue added a commit to fireblue/codex-plugin-cc that referenced this pull request May 17, 2026
@fireblue @claude
security: harden prompt-file path containment and git ref handling (o…
7015a9f 
…penai#289, openai#290)

- Reject --prompt-file paths that escape the working directory
- Use --end-of-options before user-controlled refs in all git invocations

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
tgkim-openerd pushed a commit to opnd-io/opnd-codex-plugin that referenced this pull request May 27, 2026
@RoastFried-RF
feat(security+watchdog): manual port of upstream PR openai#312 / open…
17385e4 
…ai#190 / openai#289 / openai#290 / openai#314

본 fork 의 upstream 8 HIGH cherry-pick 후보 중 Tier 1 Group B/C/D 6 PR 의
manual port. Group A (#24 + openai#311, parser robustness) 는 직전 commit fbf45c8
이 cover. `git cherry-pick` 직접 불가 (fork v2.0.0/v2.1.0 hardening 과 같은
파일 같은 영역 변경으로 dry-run 시 CONFLICT 확정) — PR diff 의 intent 를
v2.1.0 base 위에 재구현.

audit 출처: docs/upstream-tracking/2026-05-18-upstream-backlog-audit.md
Tier 1 Group B/C/D. upstream issue openai#49 (per-turn watchdog) + openai#183 partial
(finalizing only) + openai#250 partial (MCP elicitation no per-tool timeout) 자동
해결.

## Group B — per-turn watchdog (upstream PR openai#312, openai#302)

- `lib/codex.mjs`: 신규 `TurnWatchdogError` (export, exit code 124 ~
  timeout(1)), 신규 helpers `armWatchdog/disarmWatchdog/kickWatchdog`,
  `createTurnCaptureState` 에 `watchdogTimer` + `watchdogMs` field 추가.
  `captureTurn` 의 notification handler 가 매 message 마다 kick 하고,
  `runAppServerTurn` 이 `options.watchdogMs` 또는 env
  `CODEX_TURN_WATCHDOG_MS` 를 forward. 미설정 시 모든 helper no-op (opt-in).
- `codex-companion.mjs`: import 에 `TurnWatchdogError`, main `catch` 가
  `instanceof` 분기로 structured JSON line emit + exit 124. 일반 error
  경로 보존.
- `tests/turn-watchdog.test.mjs` 신규 — error class contract 2 + source
  level guard 2 (총 4 case).
- PR openai#302 (captureTurn wall-clock) 효과는 openai#312 의 inactivity watchdog 가
  super-set으로 cover (per-notification kick 가 stuck turn 검출, wall-clock
  은 long valid turn 도 잘림).

## Group C — security hardening (PR openai#190 + openai#289 + openai#290)

- **openai#190 child env sanitization** — `lib/app-server.mjs` 의
  `buildPluginCodexEnv` 가 `result = { ...baseEnv }` 로 시작해 전체 env
  전달. 신규 `sanitizePluginCodexEnv(baseEnv)` 가 inject vectors
  (`BASH_FUNC_*`, `BASH_ENV`, `ENV`, `PROMPT_COMMAND`, `LD_PRELOAD`,
  `LD_AUDIT`, `LD_LIBRARY_PATH`, `DYLD_INSERT_LIBRARIES`,
  `DYLD_LIBRARY_PATH`, `DYLD_FALLBACK_LIBRARY_PATH`, `NODE_OPTIONS`) 제거 +
  `CLAUDE_*` (단 plugin 자체 의존 `CLAUDE_PLUGIN_ROOT/CLAUDE_PLUGIN_DATA`
  보존) 제거. 사용자 의도된 보존은 `CODEX_PLUGIN_PRESERVE_ENV=...` opt-in
  comma list 로.
- **openai#289 prompt-file path containment** — `codex-companion.mjs:readTaskPrompt`
  의 `path.resolve(cwd, options["prompt-file"])` 가 path traversal 가능.
  fork user-trust 모델 보존 위해 default: cwd outside 시 stderr warn +
  계속 진행. opt-in strict: `CODEX_PLUGIN_PROMPT_FILE_STRICT=1` 시 throw.
- **openai#290 git ref `--end-of-options`** — `lib/git.mjs` 의 5 사이트
  (`buildBranchComparison` + `collectBranchContext`의 4 git invocation)
  user-controlled ref / commitRange 직접 전달 → dash-prefixed value flag
  injection 위험. 모두 `--end-of-options` 추가하여 git argv parser 의
  flag 해석 차단.

## Group D — adversarial prompt cap (PR openai#314)

- `codex-companion.mjs:buildAdversarialReviewPrompt` 에 신규 helper
  `truncateToUtf8Bytes(text, maxBytes)` (UTF-8 continuation byte 검사로
  safe cut) 추가. REVIEW_INPUT 800KB 기본 cap (Codex API 1MB 한도 대비
  safe margin). 초과 시 truncation notice 명시 + stderr warn. opt-out
  `CODEX_PLUGIN_REVIEW_PROMPT_MAX_BYTES` env.

## 검증

- `node --test tests/jsonl.test.mjs tests/turn-watchdog.test.mjs
  tests/sandbox-default-omit.test.mjs tests/finalizing-timeout.test.mjs
  tests/commands.test.mjs` — 43/43 pass
- `npm run build` exit=0 — pre-existing TS error 2건 (codex.mjs:375 +
  state.mjs:142) 본 cycle 의 typedef 갱신 영향으로 자동 해결
tgkim-openerd added a commit to opnd-io/opnd-codex-plugin that referenced this pull request May 27, 2026
@tgkim-openerd
Merge pull request #3 from opnd-io/feat/upstream-port-tier1-8high
dbbd576 
feat(upstream-port): Tier 1 8 HIGH manual port (#24 openai#311 openai#312 openai#190 openai#289 openai#290 openai#314)
tgkim-openerd pushed a commit to opnd-io/opnd-codex-plugin that referenced this pull request May 27, 2026
@RoastFried-RF
docs(changelog): sync Unreleased with PR #3-#8
a80f718 
The plugin CHANGELOG's `## Unreleased` block had entries only for the
session's first task (review-sandbox / status-watch / log-tail). PR #3-#8
landed substantial changes with no changelog record. /doc-sync adds them:

- PR #8 rename codex-plugin-cc -> opnd-codex-plugin (BREAKING invocation:
  /codex:* -> /opnd-codex:*)
- PR #3 upstream Tier 1 8-HIGH manual port (openai#312/openai#190/openai#289/openai#290/openai#314/#24/openai#311)
- PR #4 pair/capsule/output-profile/task-key workstream + ARCH-002/SEC-001
- PR #6 pair-readiness fixes A1-A4
- PR #7 upstream backlog openai#59/openai#113/openai#238/openai#75

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lohengrin332