morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #7540] [Bug] Invalid dry-run values let approved setup write client configuration #4481

New issue

Open

opened 2026-10-31 00:50:10 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:50:10 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 27, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7540

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

CLI

Observed behavior

node scripts/rea.mjs setup --yes --client codex --skill=false --dry-run=abc --format json exits 0, returns status: ready with appliedActions: ["configured_codex"], and creates the selected Codex configuration. The explicitly empty form --dry-run= has the same result. Both observations used fresh isolated homes.

The otherwise identical command with plain --dry-run returns status: planned, applies no actions, and creates no Codex configuration. --dry-run=true also preserves the plan-only behavior.

Reproduction

Use Node.js 24.22.1 and the source revision below. The verification used its existing built runtime and locked dependencies. A fresh checkout needs npm ci && npm run build:cached.

From the checkout root, run the malformed forms and the valid control with separate disposable homes:

WORK=$(mktemp -d)
for name in abc empty plain; do
  case "$name" in
    abc) option=--dry-run=abc ;;
    empty) option=--dry-run= ;;
    plain) option=--dry-run ;;
  esac
  CASE_HOME="$WORK/$name"
  mkdir -p "$CASE_HOME/config" "$CASE_HOME/data" "$CASE_HOME/cache" \
    "$CASE_HOME/state" "$CASE_HOME/temp"
  env -i PATH="$PATH" HOME="$CASE_HOME" \
    XDG_CONFIG_HOME="$CASE_HOME/config" XDG_DATA_HOME="$CASE_HOME/data" \
    XDG_CACHE_HOME="$CASE_HOME/cache" XDG_STATE_HOME="$CASE_HOME/state" \
    TMPDIR="$CASE_HOME/temp" NO_COLOR=1 \
    node scripts/rea.mjs setup --yes --client codex --skill=false \
    "$option" --format json
  echo "$name exit=$?"
  if test -f "$CASE_HOME/.codex/config.toml"; then
    echo 'Codex configuration exists'
  else
    echo 'Codex configuration absent'
  fi
done

The abc and empty cases return ready, list configured_codex, and create the configuration; plain returns planned, applies no actions, and leaves it absent.

Each malformed form reproduced twice in separate homes. Plain dry-run returned the expected plan twice. One repetition used a minimal environment with only the pinned Node directory and system tools on PATH.

Expected behavior

Reject empty or unrecognized values for the boolean dry-run option before applying setup changes. Preserve plain --dry-run and --dry-run=true as plan requests, and preserve explicit --dry-run=false as an apply request subject to approval.

The public help describes dry-run as printing a plan without applying it, and the advertised option schema is boolean. The installation guide promises a read-only plan returning planned and exit 0. Silent conversion of abc or empty text into an apply request defeats that option's stated purpose. General string-to-boolean coercion is documented by the framework; arbitrary text as a supported false literal was not documented in the inspected bundled parsing guide or README.

Impact and scope

A script passing an unset or misspelled dry-run variable can apply an approved setup operation while expecting a plan. The observed effect was a persistent Codex MCP configuration write; no data loss, provider installation, or security boundary crossing was observed. With --yes omitted, the malformed abc case returned needs_confirmation, exited 1, and created no configuration, so the approval requirement still works.

P2: the observed consequence is bounded and plain --dry-run is a demonstrated practical workaround. Observations cover setup with the Codex client on one platform; other boolean options and existing-configuration edits were not tested.

REA version

Exact node scripts/rea.mjs --version output: 6.8.0.
Source commit: 65f1c447344a423fa2785847dbcea1e9a9e2454a.

Runtime and platform

Node.js v24.22.1; macOS 27.0.1; arm64. Locked Incur 0.6.1; existing prebuilt source-checkout runtime.

Target and artifact context

No analysis target or external provider. The fixture is an initially empty, isolated home; the output artifact is a Codex MCP registration.

Interface

CLI

Earliest failure stage

Request parsing or validation

Relevant output or evidence

Public CLI results, reduced to the relevant fields:

Dry-run input	Status	Applied actions	Codex configuration
`--dry-run=abc`	`ready`	`["configured_codex"]`	created
`--dry-run=`	`ready`	`["configured_codex"]`	created
`--dry-run`	`planned`	`[]`	absent
`--dry-run=true`	`planned`	`[]`	absent
`--dry-run=false`	`ready`	`["configured_codex"]`	created

The first three rows ran twice; the last two ran once. Existence was checked on disk, and the created file contained an MCP server registration. A separate malformed abc run without --yes returned exit 1, needs_confirmation, and [], with no configuration file.

Expected-source locators: docs/installation.md:125-126, docs/cli.md:1080-1081, src/cli/setupCommands.ts:92-95, and the public setup --help / setup --schema output. The schema property is dryRun. AGENTS.md also requires planned changes and approval before setup writes.

Evidence provenance: observed public CLI behavior and filesystem effects; source-derived parser explanation below. Local verification: reproduced. Reproduction completeness: complete for isolated Codex setup.

Typed outcome or recovery expected

A nonzero parse or validation error identifying the invalid dry-run value, before configuration writes. Restoration checks should retain successful bare/true planning and approved explicit-false application. No repaired build was tested.

Analysis or suspected cause

Source-derived: the locked parser binds equals-form option text at node_modules/incur/src/Parser.ts:30-38, then converts boolean strings using value === 'true' at :322-334. Thus abc and empty text become false before schema validation. src/cliBuiltinArguments.ts:76-105 preserves these command option tokens; the surplus-positional guard does not reject them. src/cli/setupCommands.ts:272-275 forwards false and enables approval from --yes; src/application/Setup.ts:271-305 then skips the plan-only return and follows the approved apply path.

The framework's bundled SKILL.md:306-313 documents automatic coercion and equals forms, without specifying arbitrary or empty strings as accepted false values. This establishes the producing conversion; it does not establish that malformed safety-option text is an intended REA contract.

Additional context

Closed #3893 and merged PR #4068 concern misleading spaced --follow-debug-links false examples. That PR changes documentation and explicitly leaves surplus-positional handling outside its scope. This report concerns malformed equals values converted into false, so it is neither the same closure signal nor an observed regression of that documentation fix.

Open #7425, #7376, and #7093, plus closed #7430, concern numeric option coercion. Their numeric conversion branch and option-specific acceptance checks do not cover boolean dry-run. Open PR #7505 changes outline-depth's integer schema only. No exact duplicate was found in the open/closed issue, PR, and comment searches. The first affected release is unknown.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Malformed dry-run tokens produced successful, persistent client configuration writes during explicitly approved setup; no severe loss or installation was observed.
Workaround status: available
Workaround basis: Plain --dry-run returned planned with no applied actions and no Codex configuration in two isolated runs; --dry-run=true did the same once.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7540 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area CLI ### Observed behavior `node scripts/rea.mjs setup --yes --client codex --skill=false --dry-run=abc --format json` exits 0, returns `status: ready` with `appliedActions: ["configured_codex"]`, and creates the selected Codex configuration. The explicitly empty form `--dry-run=` has the same result. Both observations used fresh isolated homes. The otherwise identical command with plain `--dry-run` returns `status: planned`, applies no actions, and creates no Codex configuration. `--dry-run=true` also preserves the plan-only behavior. ### Reproduction 1. Use Node.js 24.22.1 and the source revision below. The verification used its existing built runtime and locked dependencies. A fresh checkout needs `npm ci && npm run build:cached`. 2. From the checkout root, run the malformed forms and the valid control with separate disposable homes: ```sh WORK=$(mktemp -d) for name in abc empty plain; do case "$name" in abc) option=--dry-run=abc ;; empty) option=--dry-run= ;; plain) option=--dry-run ;; esac CASE_HOME="$WORK/$name" mkdir -p "$CASE_HOME/config" "$CASE_HOME/data" "$CASE_HOME/cache" \ "$CASE_HOME/state" "$CASE_HOME/temp" env -i PATH="$PATH" HOME="$CASE_HOME" \ XDG_CONFIG_HOME="$CASE_HOME/config" XDG_DATA_HOME="$CASE_HOME/data" \ XDG_CACHE_HOME="$CASE_HOME/cache" XDG_STATE_HOME="$CASE_HOME/state" \ TMPDIR="$CASE_HOME/temp" NO_COLOR=1 \ node scripts/rea.mjs setup --yes --client codex --skill=false \ "$option" --format json echo "$name exit=$?" if test -f "$CASE_HOME/.codex/config.toml"; then echo 'Codex configuration exists' else echo 'Codex configuration absent' fi done ``` 3. The `abc` and `empty` cases return `ready`, list `configured_codex`, and create the configuration; `plain` returns `planned`, applies no actions, and leaves it absent. Each malformed form reproduced twice in separate homes. Plain dry-run returned the expected plan twice. One repetition used a minimal environment with only the pinned Node directory and system tools on PATH. ### Expected behavior Reject empty or unrecognized values for the boolean dry-run option before applying setup changes. Preserve plain `--dry-run` and `--dry-run=true` as plan requests, and preserve explicit `--dry-run=false` as an apply request subject to approval. The public help describes dry-run as printing a plan without applying it, and the advertised option schema is boolean. The installation guide promises a read-only plan returning `planned` and exit 0. Silent conversion of `abc` or empty text into an apply request defeats that option's stated purpose. General string-to-boolean coercion is documented by the framework; arbitrary text as a supported false literal was not documented in the inspected bundled parsing guide or README. ### Impact and scope A script passing an unset or misspelled dry-run variable can apply an approved setup operation while expecting a plan. The observed effect was a persistent Codex MCP configuration write; no data loss, provider installation, or security boundary crossing was observed. With `--yes` omitted, the malformed `abc` case returned `needs_confirmation`, exited 1, and created no configuration, so the approval requirement still works. P2: the observed consequence is bounded and plain `--dry-run` is a demonstrated practical workaround. Observations cover setup with the Codex client on one platform; other boolean options and existing-configuration edits were not tested. ### REA version Exact `node scripts/rea.mjs --version` output: `6.8.0`. Source commit: `65f1c447344a423fa2785847dbcea1e9a9e2454a`. ### Runtime and platform Node.js v24.22.1; macOS 27.0.1; arm64. Locked Incur 0.6.1; existing prebuilt source-checkout runtime. ### Target and artifact context No analysis target or external provider. The fixture is an initially empty, isolated home; the output artifact is a Codex MCP registration. ### Interface CLI ### Earliest failure stage Request parsing or validation ### Relevant output or evidence Public CLI results, reduced to the relevant fields: | Dry-run input | Exit | Status | Applied actions | Codex configuration | | --- | --- | --- | --- | --- | | `--dry-run=abc` | 0 | `ready` | `["configured_codex"]` | created | | `--dry-run=` | 0 | `ready` | `["configured_codex"]` | created | | `--dry-run` | 0 | `planned` | `[]` | absent | | `--dry-run=true` | 0 | `planned` | `[]` | absent | | `--dry-run=false` | 0 | `ready` | `["configured_codex"]` | created | The first three rows ran twice; the last two ran once. Existence was checked on disk, and the created file contained an MCP server registration. A separate malformed `abc` run without `--yes` returned exit 1, `needs_confirmation`, and `[]`, with no configuration file. Expected-source locators: `docs/installation.md:125-126`, `docs/cli.md:1080-1081`, `src/cli/setupCommands.ts:92-95`, and the public `setup --help` / `setup --schema` output. The schema property is `dryRun`. `AGENTS.md` also requires planned changes and approval before setup writes. Evidence provenance: observed public CLI behavior and filesystem effects; source-derived parser explanation below. Local verification: reproduced. Reproduction completeness: complete for isolated Codex setup. ### Typed outcome or recovery expected A nonzero parse or validation error identifying the invalid dry-run value, before configuration writes. Restoration checks should retain successful bare/true planning and approved explicit-false application. No repaired build was tested. ### Analysis or suspected cause Source-derived: the locked parser binds equals-form option text at `node_modules/incur/src/Parser.ts:30-38`, then converts boolean strings using `value === 'true'` at `:322-334`. Thus `abc` and empty text become false before schema validation. `src/cliBuiltinArguments.ts:76-105` preserves these command option tokens; the surplus-positional guard does not reject them. `src/cli/setupCommands.ts:272-275` forwards false and enables approval from `--yes`; `src/application/Setup.ts:271-305` then skips the plan-only return and follows the approved apply path. The framework's bundled `SKILL.md:306-313` documents automatic coercion and equals forms, without specifying arbitrary or empty strings as accepted false values. This establishes the producing conversion; it does not establish that malformed safety-option text is an intended REA contract. ### Additional context Closed [#3893](https://github.com/morluto/rea/issues/3893) and merged [PR #4068](https://github.com/morluto/rea/pull/4068) concern misleading spaced `--follow-debug-links false` examples. That PR changes documentation and explicitly leaves surplus-positional handling outside its scope. This report concerns malformed equals values converted into false, so it is neither the same closure signal nor an observed regression of that documentation fix. Open [#7425](https://github.com/morluto/rea/issues/7425), [#7376](https://github.com/morluto/rea/issues/7376), and [#7093](https://github.com/morluto/rea/issues/7093), plus closed [#7430](https://github.com/morluto/rea/issues/7430), concern numeric option coercion. Their numeric conversion branch and option-specific acceptance checks do not cover boolean dry-run. Open [PR #7505](https://github.com/morluto/rea/pull/7505) changes outline-depth's integer schema only. No exact duplicate was found in the open/closed issue, PR, and comment searches. The first affected release is unknown. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Malformed dry-run tokens produced successful, persistent client configuration writes during explicitly approved setup; no severe loss or installation was observed. - Workaround status: available - Workaround basis: Plain --dry-run returned planned with no applied actions and no Codex configuration in two isolated runs; --dry-run=true did the same once.

bsakaguchi referenced this issue

2026-10-31 01:13:09 +00:00

[PR #4481] [CLOSED] fix(reconstruction): report ledger caller-input mistakes as invalid input #6266

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#4481

No description provided.

Rows
Columns