morluto/rea

Fork 0

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

[PR #3418] [MERGED] fix(process): keep observed symlink targets in filesystem snapshots #5732

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3418
Author: @Sriram-PR
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @baron

Base: main ← Head: fix/process-symlink-target-spelling

📝 Commits (1)

902f3f2 fix(process): keep observed symlink targets in filesystem snapshots

📊 Changes

2 files changed (+35 additions, -2 deletions)

View changed files

📝 src/process/capture/FilesystemCoverage.test.ts (+32 -0)
📝 src/process/capture/FilesystemSnapshot.ts (+3 -2)

📄 Description

Summary

capture-process reported a symlink as unchanged after the captured command retargeted it. Filesystem snapshots stored resolve(dirname(link), readlink(link)), a lexical absolute path, so different targets like aaa/../target and bbb/../target collapsed to the same symlink_target. With equal-length targets the size matched too, and the before/after comparison saw no change, even though the new link is broken.

Fixes #3303

Problem and expected behavior

Smallest trigger: link -> aaa/../target (with aaa/ present, so the link works), replaced during capture by link -> bbb/../target (no bbb/, so the link is broken). Expected: the snapshots keep the two observed targets and root_0:link is modified.

Change and scope

src/process/capture/FilesystemSnapshot.ts now stores the target exactly as readlink returns it. The comparison in ProcessFilesystemEffects.ts is unchanged; it now compares the observed targets.
symlink_target is the only consumer-facing change: relative targets are reported as written (aaa/../target) instead of as a lexically resolved absolute path. Absolute targets are unchanged (the lifecycle boundary test's /etc/passwd escape still passes).
No traversal or extra filesystem access is added; the snapshot still only lstats and readlinks the link.

Contribution scope: ordinary fix within existing boundaries; no scope decision needed.

Contract and boundary impact

Semantic owner and earliest changed stage: snapshotRoots in src/process/capture/FilesystemSnapshot.ts, when a symlink entry is recorded.
Contribution scope fit and linked maintainer decision: not applicable (bug fix, #3303).
CLI and MCP/tool-catalog contract: no schema change (symlink_target is still a string). Its value is now the link's stored target rather than a resolved absolute path for relative links.
Provider, bridge, target-format, or platform compatibility: POSIX symlinks; Windows tests for symlinks are skipped as before.
Evidence, artifact, provenance, or reconstruction contract: retargeted links whose old and new targets resolve to the same lexical path are now reported as modified; relative targets no longer embed the observed root's absolute path.
Target selection, authority, upstream behavior, and cleanup details: not applicable; capture execution is unchanged.
Generated metadata, package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

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.

Evidence and regression coverage

Tests added or updated: src/process/capture/FilesystemCoverage.test.ts, a snapshot/compare test that retargets link from aaa/../target to the equal-length bbb/../target and expects both raw targets in the snapshots and modified for the link.
Base reproduction: on the base revision the test fails with expected '/tmp/rea-fs-symlink-spelling-.../target' to be 'aaa/../target'.
User-visible CLI output: capture-process with /bin/sh -c 'rm link && ln -s bbb/../target link' in a directory where link -> aaa/../target, normalization.paths: false:
```
base: {"link_effect":"unchanged","targets":["<root>/target"],"truncated":false}
PR:   {"link_effect":"modified","targets":["aaa/../target","bbb/../target"],"truncated":false}
```
readlink link afterwards prints bbb/../target and the link is broken, matching the report.
Remaining proof gaps: ran on Linux x64 only (the report reproduced on macOS); not run on Windows.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:local -- src/process/capture/FilesystemCoverage.test.ts — 15 passed (new test fails on the base revision)
npm run test:focused on 33 process-capture test files (src/process/capture/*.test.ts, src/domain/process/*.test.ts, and the tests referencing symlink_target, snapshotRoots or capture-process, including tests/boundary/process/processCaptureLifecycle.test.ts) — 281 passed, 4 skipped
npm run check:fast — typecheck and lint passed
CLI comparison above, built with npm run build:cached on base and on this branch

Compatibility, safety, and release

Breaking changes or migration steps: none to the schema; consumers that assumed symlink_target was always absolute will now see relative targets as written.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; macOS and Windows not run.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none added; relative targets no longer include the observed root's absolute path.

Author readiness

Human owner: @Sriram-PR

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

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/3418 **Author:** [@Sriram-PR](https://github.com/Sriram-PR) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@baron](https://github.com/baron) **Base:** `main` ← **Head:** `fix/process-symlink-target-spelling` --- ### 📝 Commits (1) - [`902f3f2`](https://github.com/morluto/rea/commit/902f3f29a954f6be61e31151831ad8a9bed16a8c) fix(process): keep observed symlink targets in filesystem snapshots ### 📊 Changes **2 files changed** (+35 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/process/capture/FilesystemCoverage.test.ts` (+32 -0) 📝 `src/process/capture/FilesystemSnapshot.ts` (+3 -2) </details> ### 📄 Description ## Summary `capture-process` reported a symlink as `unchanged` after the captured command retargeted it. Filesystem snapshots stored `resolve(dirname(link), readlink(link))`, a lexical absolute path, so different targets like `aaa/../target` and `bbb/../target` collapsed to the same `symlink_target`. With equal-length targets the size matched too, and the before/after comparison saw no change, even though the new link is broken. Fixes #3303 ## Problem and expected behavior Smallest trigger: `link -> aaa/../target` (with `aaa/` present, so the link works), replaced during capture by `link -> bbb/../target` (no `bbb/`, so the link is broken). Expected: the snapshots keep the two observed targets and `root_0:link` is `modified`. ## Change and scope - `src/process/capture/FilesystemSnapshot.ts` now stores the target exactly as `readlink` returns it. The comparison in `ProcessFilesystemEffects.ts` is unchanged; it now compares the observed targets. - `symlink_target` is the only consumer-facing change: relative targets are reported as written (`aaa/../target`) instead of as a lexically resolved absolute path. Absolute targets are unchanged (the lifecycle boundary test's `/etc/passwd` escape still passes). - No traversal or extra filesystem access is added; the snapshot still only `lstat`s and `readlink`s the link. Contribution scope: ordinary fix within existing boundaries; no scope decision needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `snapshotRoots` in `src/process/capture/FilesystemSnapshot.ts`, when a symlink entry is recorded. - Contribution scope fit and linked maintainer decision: not applicable (bug fix, #3303). - CLI and MCP/tool-catalog contract: no schema change (`symlink_target` is still a string). Its value is now the link's stored target rather than a resolved absolute path for relative links. - Provider, bridge, target-format, or platform compatibility: POSIX symlinks; Windows tests for symlinks are skipped as before. - Evidence, artifact, provenance, or reconstruction contract: retargeted links whose old and new targets resolve to the same lexical path are now reported as `modified`; relative targets no longer embed the observed root's absolute path. - Target selection, authority, upstream behavior, and cleanup details: not applicable; capture execution is unchanged. - Generated metadata, package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] 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. ## Evidence and regression coverage - Tests added or updated: `src/process/capture/FilesystemCoverage.test.ts`, a snapshot/compare test that retargets `link` from `aaa/../target` to the equal-length `bbb/../target` and expects both raw targets in the snapshots and `modified` for the link. - Base reproduction: on the base revision the test fails with `expected '/tmp/rea-fs-symlink-spelling-.../target' to be 'aaa/../target'`. - User-visible CLI output: `capture-process` with `/bin/sh -c 'rm link && ln -s bbb/../target link'` in a directory where `link -> aaa/../target`, `normalization.paths: false`: ``` base: {"link_effect":"unchanged","targets":["<root>/target"],"truncated":false} PR: {"link_effect":"modified","targets":["aaa/../target","bbb/../target"],"truncated":false} ``` `readlink link` afterwards prints `bbb/../target` and the link is broken, matching the report. - Remaining proof gaps: ran on Linux x64 only (the report reproduced on macOS); not run on Windows. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/process/capture/FilesystemCoverage.test.ts` — 15 passed (new test fails on the base revision) - `npm run test:focused` on 33 process-capture test files (`src/process/capture/*.test.ts`, `src/domain/process/*.test.ts`, and the tests referencing `symlink_target`, `snapshotRoots` or `capture-process`, including `tests/boundary/process/processCaptureLifecycle.test.ts`) — 281 passed, 4 skipped - `npm run check:fast` — typecheck and lint passed - CLI comparison above, built with `npm run build:cached` on base and on this branch ## Compatibility, safety, and release - Breaking changes or migration steps: none to the schema; consumers that assumed `symlink_target` was always absolute will now see relative targets as written. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; macOS and Windows not run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none added; relative targets no longer include the observed root's absolute path. ## Author readiness Human owner: @Sriram-PR - [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] 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:09:01 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:10 +00:00

[PR #5803] [MERGED] fix(artifacts): admit ZIP archives after unclaimed leading bytes #7073

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

No description provided.

Rows
Columns