morluto/rea

Fork 0

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

[PR #2979] [CLOSED] fix(install): validate empty version requests and unreported installed versions #5534

New issue

Closed

opened 2026-10-31 01:07:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2979
Author: @555aaditya
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/2580-installer-version-validation

📝 Commits (3)

11369c9 fix(install): validate empty version requests and unreported installed versions
ac7bbd1 Merge remote-tracking branch 'origin/main' into HEAD
e41c436 Merge remote-tracking branch 'origin/main' into fix/2580-installer-version-validation

📊 Changes

1 file changed (+24 additions, -10 deletions)

View changed files

📝 tests/boundary/filesystem/installScript.test.ts (+24 -10)

📄 Description

Summary

Fixes #2580.
Fixes #2581.

Two small version-identity defects in install.sh, both in how the installer reads a version string:

#2580, empty explicit request. --version "" was indistinguishable from omitting the option, because validation was gated on [ -n "$version" ]. The installer silently resolved the latest release and installed it, instead of rejecting a request that isn't a semantic version. It now records that --version was supplied and validates the value like any other explicit version. Omitting the option, or leaving REA_VERSION empty, still resolves the latest release.
#2581, installed command reports no version. If the installed rea --version exited 0 but printed nothing (or only whitespace), the read "succeeded" with an empty string. The installer then reported installed version does not match 0.3.0, the mismatch diagnostic with a blank actual value. Empty output now takes the documented unreadable-version route instead. A different non-empty version still fails as a mismatch.

Both fixes keep a single validation point and reuse the existing messages, so docs/user-visible-errors.csv needs no new rows.

Problem and expected behavior

Invocation	Before	After
`install.sh --version "" --dry-run`	exit 0, `Version: <latest>`, `Dry run complete`	exit 1, `version must be an exact semantic version.`, no plan printed, no npm call
`install.sh --version not-a-version`	exit 1, semver error	unchanged
`install.sh --version 0.3.0`	plan for 0.3.0	unchanged
`install.sh` (or `REA_VERSION=""`)	resolves latest	unchanged
installed `rea --version` prints `""` / `" "`	`installed version does not match 0.3.0. …`	`the installed REA version could not be read. Reinstall the requested version, then retry.`
installed `rea --version` prints `9.9.9`	mismatch	unchanged
installed `rea --version` exits 1	unreadable	unchanged

Violated invariants:

docs/installation.md documents --version <semver>, and docs/user-visible-errors.csv (install.sh "version syntax") says explicit versions that aren't exact semantic versions are rejected.
The same CSV routes "installed rea executable cannot report its version" to the unreadable message, and uses the mismatch message only when a different version is reported.

Change and scope

#2580: a version_requested flag is set where --version is parsed. The existing validation branch now runs when the option was given or when REA_VERSION is non-empty. I chose this over a separate empty-value check in the argument parser so that there is still exactly one place deciding whether an explicit version is valid, with one message.
#2581: the version read becomes installed_version="$(…)" && [ -n "$installed_version" ] || fail "…could not be read…". A nonzero exit (the existing route) and empty output now share the one unreadable-version failure, and the following mismatch check only ever sees a non-empty version.

Both changes stay inside the truncation-guard brace group and use the existing fail helper. The script still parses with bash -n.

Not included: an empty REA_VERSION keeps meaning "not set", which is the existing behaviour and is preserved by #2580's restoration check.

Contribution scope: an ordinary fix within existing installer behaviour, so a maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: install.sh, in option parsing and version validation (#2580), and the post-install identity read (#2581).
Contribution scope fit and linked maintainer decision: bug fix; not applicable.
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none. Bash constructs only, used in the same style as the surrounding script.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: an empty explicit request now fails before any release lookup, install plan, or npm call. Nothing else about what gets installed changes.
Generated metadata, package, or installation impact: installer behaviour only for the two edge cases above. No package metadata change.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

(No analysis-target interaction. The installer's existing network and npm behaviour is unchanged, except that an empty explicit version now stops before reaching either.)

Evidence and regression coverage

All in tests/boundary/filesystem/installScript.test.ts, using the existing fake curl, npm and rea fixture:

#2580: "" added to the existing rejects malformed semantic versions before invoking npm cases. It asserts empty stdout (no plan), the exact semver error, and no npm invocation.
#2580 control: new resolves the latest release when REA_VERSION is empty. An empty environment variable still selects the latest tag (Version: 0.3.0).
#2581: new installer installed version identity block, reports an installed command without a version as unreadable for "" and " ". It asserts the exact unreadable-version message.
The existing unreadable installed version (nonzero exit) and version mismatch (9.9.9) table rows remain the controls for the other two routes.
Fixture: the fake rea now prints ${FAKE_REA_VERSION-0.3.0} instead of ${FAKE_REA_VERSION:-0.3.0}, so a test can request a genuinely empty version. When unset it still defaults to 0.3.0, and no existing test sets it to an empty value.
I put the installed-version cases in their own small describe, rather than as two more rows in the curl installer scenarios table, to keep that callback under the max-lines-per-function limit.

Base reproduction: with only the install.sh change reverted, the three new cases fail (3 failed | 34 passed). With it, all 37 pass.

Remaining proof gaps: verified with the repo's fake-tool fixture, not a real curl | bash install. Linux behaviour is covered only through the same fixture (FAKE_PLATFORM), via CI.

Validation performed

All run on macOS (arm64), Bash at /bin/bash, Node 26.5.0, npm 11.17.0, with locked dependencies, rebased on current origin/main:

npx vitest run tests/boundary/filesystem/installScript.test.ts: with the install.sh fix reverted, 3 failed (the new cases) and 34 passed. With the fix, 37 passed.
bash -n install.sh: OK.
npm run check:fast (typecheck + lint + module boundaries): passed, also via the pre-push hook.
npx oxfmt --check and npx oxlint on the changed files: clean.
I didn't run shellcheck; it isn't installed locally.

Compatibility, safety, and release

Breaking changes or migration steps: none for valid use. Scripts that pass --version "" expecting "latest" must drop the option, or set REA_VERSION to empty or leave it unset. That invocation previously contradicted the documented --version <semver> contract.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the installer now fails earlier (before any network request) for an empty explicit version, and keeps failing closed on an unreadable installed version. No new commands, paths, or inputs.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2979 **Author:** [@555aaditya](https://github.com/555aaditya) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/2580-installer-version-validation` --- ### 📝 Commits (3) - [`11369c9`](https://github.com/morluto/rea/commit/11369c96ba5a9f4e4591e3d973833aa7f17fdbf1) fix(install): validate empty version requests and unreported installed versions - [`ac7bbd1`](https://github.com/morluto/rea/commit/ac7bbd14af0fe9ea6f50ea9e00b109fa8a6e41fc) Merge remote-tracking branch 'origin/main' into HEAD - [`e41c436`](https://github.com/morluto/rea/commit/e41c436fd6640bf477aed0a03fecc3ccebfc62aa) Merge remote-tracking branch 'origin/main' into fix/2580-installer-version-validation ### 📊 Changes **1 file changed** (+24 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/filesystem/installScript.test.ts` (+24 -10) </details> ### 📄 Description ## Summary Fixes #2580. Fixes #2581. Two small version-identity defects in `install.sh`, both in how the installer reads a version string: 1. **#2580, empty explicit request.** `--version ""` was indistinguishable from omitting the option, because validation was gated on `[ -n "$version" ]`. The installer silently resolved the latest release and installed it, instead of rejecting a request that isn't a semantic version. It now records that `--version` was supplied and validates the value like any other explicit version. Omitting the option, or leaving `REA_VERSION` empty, still resolves the latest release. 2. **#2581, installed command reports no version.** If the installed `rea --version` exited 0 but printed nothing (or only whitespace), the read "succeeded" with an empty string. The installer then reported `installed version does not match 0.3.0`, the mismatch diagnostic with a blank actual value. Empty output now takes the documented unreadable-version route instead. A different non-empty version still fails as a mismatch. Both fixes keep a single validation point and reuse the existing messages, so `docs/user-visible-errors.csv` needs no new rows. ## Problem and expected behavior | Invocation | Before | After | |---|---|---| | `install.sh --version "" --dry-run` | exit 0, `Version: <latest>`, `Dry run complete` | exit 1, `version must be an exact semantic version.`, no plan printed, no npm call | | `install.sh --version not-a-version` | exit 1, semver error | unchanged | | `install.sh --version 0.3.0` | plan for 0.3.0 | unchanged | | `install.sh` (or `REA_VERSION=""`) | resolves latest | unchanged | | installed `rea --version` prints `""` / `" "` | `installed version does not match 0.3.0. …` | `the installed REA version could not be read. Reinstall the requested version, then retry.` | | installed `rea --version` prints `9.9.9` | mismatch | unchanged | | installed `rea --version` exits 1 | unreadable | unchanged | **Violated invariants:** - `docs/installation.md` documents `--version <semver>`, and `docs/user-visible-errors.csv` (install.sh "version syntax") says explicit versions that aren't exact semantic versions are rejected. - The same CSV routes "installed rea executable cannot report its version" to the unreadable message, and uses the mismatch message only when a *different* version is reported. ## Change and scope - **#2580:** a `version_requested` flag is set where `--version` is parsed. The existing validation branch now runs when the option was given *or* when `REA_VERSION` is non-empty. I chose this over a separate empty-value check in the argument parser so that there is still exactly one place deciding whether an explicit version is valid, with one message. - **#2581:** the version read becomes `installed_version="$(…)" && [ -n "$installed_version" ] || fail "…could not be read…"`. A nonzero exit (the existing route) and empty output now share the one unreadable-version failure, and the following mismatch check only ever sees a non-empty version. Both changes stay inside the truncation-guard brace group and use the existing `fail` helper. The script still parses with `bash -n`. Not included: an empty `REA_VERSION` keeps meaning "not set", which is the existing behaviour and is preserved by #2580's restoration check. Contribution scope: an ordinary fix within existing installer behaviour, so a maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `install.sh`, in option parsing and version validation (#2580), and the post-install identity read (#2581). - Contribution scope fit and linked maintainer decision: bug fix; not applicable. - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. Bash constructs only, used in the same style as the surrounding script. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: an empty explicit request now fails before any release lookup, install plan, or npm call. Nothing else about what gets installed changes. - Generated metadata, package, or installation impact: installer behaviour only for the two edge cases above. No package metadata change. Expected effects: - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. (No analysis-target interaction. The installer's existing network and npm behaviour is unchanged, except that an empty explicit version now stops before reaching either.) ## Evidence and regression coverage All in `tests/boundary/filesystem/installScript.test.ts`, using the existing fake curl, npm and rea fixture: - **#2580:** `""` added to the existing `rejects malformed semantic versions before invoking npm` cases. It asserts empty stdout (no plan), the exact semver error, and no npm invocation. - **#2580 control:** new `resolves the latest release when REA_VERSION is empty`. An empty environment variable still selects the latest tag (`Version: 0.3.0`). - **#2581:** new `installer installed version identity` block, `reports an installed command without a version as unreadable` for `""` and `" "`. It asserts the exact unreadable-version message. - The existing `unreadable installed version` (nonzero exit) and `version mismatch` (`9.9.9`) table rows remain the controls for the other two routes. - **Fixture:** the fake `rea` now prints `${FAKE_REA_VERSION-0.3.0}` instead of `${FAKE_REA_VERSION:-0.3.0}`, so a test can request a genuinely empty version. When unset it still defaults to `0.3.0`, and no existing test sets it to an empty value. - I put the installed-version cases in their own small `describe`, rather than as two more rows in the `curl installer scenarios` table, to keep that callback under the `max-lines-per-function` limit. **Base reproduction:** with only the `install.sh` change reverted, the three new cases fail (`3 failed | 34 passed`). With it, all 37 pass. **Remaining proof gaps:** verified with the repo's fake-tool fixture, not a real `curl | bash` install. Linux behaviour is covered only through the same fixture (`FAKE_PLATFORM`), via CI. ## Validation performed All run on macOS (arm64), Bash at `/bin/bash`, Node 26.5.0, npm 11.17.0, with locked dependencies, rebased on current `origin/main`: - `npx vitest run tests/boundary/filesystem/installScript.test.ts`: with the `install.sh` fix reverted, 3 failed (the new cases) and 34 passed. With the fix, 37 passed. - `bash -n install.sh`: OK. - `npm run check:fast` (typecheck + lint + module boundaries): passed, also via the pre-push hook. - `npx oxfmt --check` and `npx oxlint` on the changed files: clean. - I didn't run `shellcheck`; it isn't installed locally. ## Compatibility, safety, and release - Breaking changes or migration steps: none for valid use. Scripts that pass `--version ""` expecting "latest" must drop the option, or set `REA_VERSION` to empty or leave it unset. That invocation previously contradicted the documented `--version <semver>` contract. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the installer now fails earlier (before any network request) for an empty explicit version, and keeps failing closed on an unreadable installed version. No new commands, paths, or inputs. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:07:23 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:25 +00:00

[PR #5574] [CLOSED] fix(javascript): keep unavailable analysis scopes unknown in version comparison #6973

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#5534

No description provided.

Rows
Columns