[PR #4335] [MERGED] fix(setup): reject NUL bytes in registration environments #6180

New issue

Closed

opened 2026-10-31 01:12:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4335
Author: @ShuaixinHuang
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @morluto

Base: main ← Head: codex/fix-registration-nul-environment

📝 Commits (2)

7504b24 fix(setup): reject NUL bytes in registration environments
2e8b0fa Merge remote-tracking branch 'origin/main' into codex/fix-registration-nul-environment

📊 Changes

4 files changed (+206 additions, -3 deletions)

View changed files

📝 src/application/DshCordisPatch.ts (+1 -1)
📝 src/config.test.ts (+32 -0)
📝 src/config/environment.ts (+24 -1)
📝 tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts (+149 -1)

📄 Description

Summary

Reject NUL bytes in registration environment names and values before known-setting parsing drops custom keys. doctor reports configuration drift; setup --yes reports needs_human without writing configuration or backup files. Ordinary custom strings, empty values and repaired registrations remain accepted; rejected values are omitted from diagnostics.

The shared environment parser owns byte admission. DSH forwards unresolved expression names to that owner without evaluating expressions. Four changed files; no PR-specific dependency, schema, provider or generated-metadata changes. No target execution or new capability.

Executed evidence

Latest synchronized head: 2e8b0fa6cf9dac516c5c2329a47a0998df13ad98, including upstream main 8ab5024 and its shared browser/MCP lifecycle fixes. Pinned Windows Node 24.18.0/npm 11.16.0 and matching locked dependencies.

Original baseline: seven regression failures, plus an unresolved-expression-name case that fails an incomplete literal-only fix.
Current-head config/public-CLI selection: 80 passed / 3 failed. All five real setup/doctor NUL scenarios pass, including DSH expression names. Failures are two Hopper launcher platform expectations and an OpenCode merged-registration readiness expectation.
All three failures were reproduced with current upstream production in a separate checkout without this NUL change: 3 failed / 71 name-filtered. The full selected suite is not claimed green on Windows.
Current-head npm run check: 6/6 tasks and architecture guards passed. git diff --check passed.
Earlier pre-sync revision: 138 related environment/setup tests passed, and the dedicated Windows package verifier passed (207 real stdio-MCP tool contracts plus setup/idempotence/uninstall/update). These are historical, not reruns on the synchronized head. Generic package verification previously stopped on Windows symlink EPERM.

No external MCP client or real analysis engine was exercised. Current-head GitHub CI passed, including all five Linux shards and the required aggregate. Codex review completed with no findings; Greptile check passed.

Author readiness

Human owner: @ShuaixinHuang

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

One concrete reproduced defect, with shared producing-boundary validation and related DSH handling.
Normal custom values and repaired registrations remain accepted; invalid setup is non-writing.
Executed checks, failed attempts and unavailable coverage are reported above.
Human owner declares initial readiness.
Required current-head CI passes.

_{🔄 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/4335 **Author:** [@ShuaixinHuang](https://github.com/ShuaixinHuang) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-registration-nul-environment` --- ### 📝 Commits (2) - [`7504b24`](https://github.com/morluto/rea/commit/7504b24f3ee9fcdb18565cca147715dce0f62cf7) fix(setup): reject NUL bytes in registration environments - [`2e8b0fa`](https://github.com/morluto/rea/commit/2e8b0fa6cf9dac516c5c2329a47a0998df13ad98) Merge remote-tracking branch 'origin/main' into codex/fix-registration-nul-environment ### 📊 Changes **4 files changed** (+206 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/application/DshCordisPatch.ts` (+1 -1) 📝 `src/config.test.ts` (+32 -0) 📝 `src/config/environment.ts` (+24 -1) 📝 `tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts` (+149 -1) </details> ### 📄 Description ## Summary Reject NUL bytes in registration environment names and values before known-setting parsing drops custom keys. `doctor` reports configuration drift; `setup --yes` reports `needs_human` without writing configuration or backup files. Ordinary custom strings, empty values and repaired registrations remain accepted; rejected values are omitted from diagnostics. The shared environment parser owns byte admission. DSH forwards unresolved expression names to that owner without evaluating expressions. Four changed files; no PR-specific dependency, schema, provider or generated-metadata changes. No target execution or new capability. ## Executed evidence Latest synchronized head: `2e8b0fa6cf9dac516c5c2329a47a0998df13ad98`, including upstream main `8ab5024` and its shared browser/MCP lifecycle fixes. Pinned Windows Node 24.18.0/npm 11.16.0 and matching locked dependencies. - Original baseline: seven regression failures, plus an unresolved-expression-name case that fails an incomplete literal-only fix. - Current-head config/public-CLI selection: **80 passed / 3 failed**. All five real setup/doctor NUL scenarios pass, including DSH expression names. Failures are two Hopper launcher platform expectations and an OpenCode merged-registration readiness expectation. - All three failures were reproduced with current upstream production in a separate checkout without this NUL change: **3 failed / 71 name-filtered**. The full selected suite is not claimed green on Windows. - Current-head `npm run check`: **6/6 tasks and architecture guards passed**. `git diff --check` passed. - Earlier pre-sync revision: 138 related environment/setup tests passed, and the dedicated Windows package verifier passed (207 real stdio-MCP tool contracts plus setup/idempotence/uninstall/update). These are historical, not reruns on the synchronized head. Generic package verification previously stopped on Windows symlink EPERM. No external MCP client or real analysis engine was exercised. Current-head [GitHub CI passed](https://github.com/morluto/rea/actions/runs/38855368062), including all five Linux shards and the required aggregate. Codex review completed with no findings; Greptile check passed. ## Author readiness Human owner: @ShuaixinHuang - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] One concrete reproduced defect, with shared producing-boundary validation and related DSH handling. - [x] Normal custom values and repaired registrations remain accepted; invalid setup is non-writing. - [x] Executed checks, failed attempts and unavailable coverage are reported above. - [x] Human owner declares initial readiness. - [x] Required current-head CI passes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:28 +00:00