Skip to content

fix(desktop): keep memory imports inside source root - #744

Open
imMamdouhaboammar wants to merge 1 commit into
lsdefine:mainfrom
imMamdouhaboammar:fix/memory-import-symlink-boundary-upstream
Open

fix(desktop): keep memory imports inside source root#744
imMamdouhaboammar wants to merge 1 commit into
lsdefine:mainfrom
imMamdouhaboammar:fix/memory-import-symlink-boundary-upstream

Conversation

@imMamdouhaboammar

Copy link
Copy Markdown

Problem

The desktop memory-import path copies files from another GenericAgent directory into the current memory/ and temp/model_responses/ trees.

_import_memory_from() previously iterated those trees with Path.rglob() and passed every non-directory entry directly to shutil.copy2(). Because copy2() follows source symlinks by default, a backup containing a symlink can make the import copy the contents of a file outside the selected backup directory into GenericAgent's local memory or response history.

Fix

Add one shared _safe_import_items() boundary that:

  • resolves the selected source root once
  • rejects entries whose resolved path leaves that source root
  • skips symlink entries themselves
  • lets filesystem resolution errors propagate instead of silently returning a partial ok=True restore
  • yields normal files and directories unchanged

Both memory/ and model_responses/ imports use the same iterator.

Verification

TDD and review-driven regression work was performed before rebuilding this clean branch:

  1. Regression-only run 31163257090 failed exactly on the two external file-symlink cases while regular-file import passed
  2. The shared source-boundary fix passed run 31163337902
  3. External directory-symlink coverage passed persisted-state run 31163422371
  4. Review feedback identified that OSError from path resolution was silently swallowed. Regression-only run 31164088693 passed all existing cases and failed only because the expected OSError was not raised
  5. The minimal error-handling fix passed run 31164242854: compile, all 5 regression methods, and git diff --check
  6. The test harness was also isolated from global sys.path mutation and its temporary GA root now has explicit module cleanup

Final coverage verifies external file symlinks, external directory symlinks, model-response symlinks, filesystem resolution errors, and normal file imports.

Scope

  • 1 production file modified
  • 1 focused regression-test file added
  • 1 clean commit on top of current main
  • no dependency or configuration changes

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant