Skip to content

fix: refuse relative and re-listed folders in Link checkout - #89

Merged
HemSoft merged 2 commits into
mainfrom
fix/issue-87-normalize-link-checkout-folder
Nov 6, 2026
Merged

HemSoft merged 2 commits into
mainfrom
fix/issue-87-normalize-link-checkout-folder

Conversation

@HemSoft

@HemSoft HemSoft commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Risk level: 2/10 (project-dashboard)
Worked on by: claude-opus-5-5 on Laptop, for initiating Home70828f7a

Summary

Link checkout now checks the typed folder before it reads or writes anything:

  • Relative folder: refused with "Type the checkout's full path, not a relative folder; nothing was written." This is checked first, so the result never depends on the working folder the dashboard started in.
  • Missing folder: "That folder doesn't exist; nothing was written." Before, this showed the "no GitHub origin" message.
  • Already listed under another spelling: the folder is resolved (std::fs::canonicalize) and compared with each full-path entry in config.toml. A .. spelling, a case-only spelling on Windows, or a link to a listed checkout writes nothing and returns the listed path, so the table gets no second row.
  • What gets saved: still the path as typed. The resolved path is used only for comparison, so config.toml never gets a \\?\ prefix.

The change is in projects::link_checkout (crates/dashboard-core/src/projects.rs). config.rs is unchanged, so #88 can follow without conflicts. No git process is started (tests/no_child_process.rs still passes).

Acceptance Criteria

  • A relative folder is refused with a full-path message, and Link checkout never writes a relative entry to config.toml: a_relative_or_missing_folder_is_not_linked_and_nothing_is_written covers tickdown and ./tickdown, plus \tickdown and C:tickdown on Windows. The file is unchanged byte for byte. The test changes no working directory.
  • A .. or case-only (case-insensitive file system) spelling of a listed checkout writes nothing and adds no row: another_spelling_of_a_listed_checkout_adds_no_second_row covers sub/../tickdown, the upper-cased path (Windows) and a symlink (Unix). The file, with its comment, is unchanged and repositories keeps one entry.
  • The exact listed path, or that path with a trailing separator, still writes nothing: covered by the same test.
  • A new absolute folder with the right origin is still appended once, with comments kept: linking_a_checkout_appends_it_and_keeps_every_comment and single_line_empty_and_missing_lists_are_edited_too are unchanged and pass. The UI test link_checkout_checks_the_origin_and_appends_to_config_toml passes too.
  • Saved paths keep a normal form on Windows (no \\?\ prefix): a_new_checkout_is_saved_as_typed_and_found_again_under_another_spelling saves sub/../tickdown as typed, asserts there's no \\?\, and then finds it again from the plain path without writing.

Definition of Done

  • Fix in dashboard-core, with behavior-named tests for the relative, .., case-only (Windows), exact and trailing-separator cases: three new tests in crates/dashboard-core/tests/projects.rs. Run against the old link_checkout, all three fail (3 failed); with the fix, 17/17 pass.
  • Failure paths: a relative or missing folder writes nothing (asserted).
  • fmt, clippy, workspace tests and advisories: all pass on Linux at c7d3447 (see Validation).
  • Windows full gates: both required Windows jobs passed on c7d3447 in CI 42261815690. Local full gates remain unrun after the disk-full failure.
  • Clean current-head AI review: SFL PASS round 1 on c7d3447; one non-blocking Low note, zero unresolved threads.

Validation

At head c7d344790c6c96b223fe46f694055990c155ab64, on Mini (Linux), from a git archive of that commit:

  • cargo fmt --all --check: pass
  • cargo clippy --workspace --all-targets --locked -- -D warnings: pass
  • cargo test --workspace --locked: pass (dashboard-app 184 passed, 14 ignored; every dashboard-core suite passed, including projects 17/17 and no_child_process)
  • cargo deny --locked check advisories: advisories ok

On Laptop (Windows 11, git 2.55), cargo test -p dashboard-core --locked --test projects: 17 passed, including the case-only and C:tickdown cases. The only later edit to that file was a clippy fix (std::slice::from_ref) and an exact \\?\ literal. Linux reran it at the final head.

Screenshots come from the existing headless capture test, with synthetic temporary folders and fake pmd data. No window opens and no real profile is used:
CAPTURE_DIR=<abs dir> cargo test -p dashboard-app --locked captures -- --ignored --exact captures::captures (Linux headless renderer, 1100×760 at 200% device scale). The capture now opens Manage > Projects > hs-buddy, which has no checkout, and presses Link checkout three times:

Relative folder hs-buddy: refused, nothing written.

Link checkout refuses the relative folder hs-buddy with the full-path message

Missing folder /tmp/…/missing: says it doesn't exist.

Link checkout reports that the missing folder doesn't exist

/tmp/…/sub/../hs-buddy, typed after /tmp/…/hs-buddy was added to config.toml behind the open dialog: the success line names the listed path, not the typed one. The capture asserts config.toml is byte-for-byte unchanged ("no second row").

Link checkout reports the already-listed hs-buddy path for a dot-dot spelling and writes nothing

No recording: each state is the single frame after one button press, and the headless renderer writes still frames only.

Risks and deferred work

  • Windows-only regression still unrun at the final head: the Laptop's C: drive had 0 MB free during the Windows workspace clippy/test run, so it stopped before finishing. No source failure was reported. The focused Windows --test projects run passed before the last test-only edit (a clippy fix and an exact \\?\ literal); the Windows clippy --workspace and test --workspace at c7d3447 have not been run locally. Both Windows jobs in CI 42261815690 passed at the final head and supply that Windows evidence.
  • Success message: for an already-listed checkout, the existing message "Linked . The dashboard adds it to the table." still shows. The row is already in the table; the wording is unchanged.
  • Unreachable entry: if a listed entry is on an unreachable network share, resolving it can take as long as the OS timeout. This runs on the background executor, as the identity read already does.
  • Discover separate-git-dir checkouts on first run #88 (first-run discovery of separate-git-dir checkouts) is separate.

Closes #87

🤖 Generated with Claude Code

Current-head delivery gate: all five required CI jobs passed on c7d3447; SFL PASS names that exact head, all review threads are resolved, and effective rules require no formal approval.

HemSoft and others added 2 commits November 6, 2026 12:16
Link checkout saved the typed folder as is and treated it as listed only when
it was the same PathBuf. A relative folder then resolved against wherever the
dashboard started, and a `..` or case-only spelling of a listed checkout added
a second row.

Refuse a folder that isn't a full path before anything is read, say when it
doesn't exist, and compare the resolved folder with each full-path entry, so
another spelling of a listed checkout writes nothing. The typed path is still
what gets saved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@HemSoft

HemSoft commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@sfl review

@HemSoft HemSoft added the model:claude-opus-5-5 PR authored by claude-opus-5-5 label Nov 6, 2026

@hemsoft-sfl-reviewer hemsoft-sfl-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Verdict: PASS • Round 1

Confidence: 8/10

Review done by claude-opus-5-5

The PR changes projects::link_checkout so it refuses a relative folder, reports a missing or unopenable folder, and compares the canonicalized folder with each absolute config.toml entry so another spelling of a listed checkout writes nothing. It also adds core tests and capture states. I found no evidenced Critical, High or Medium defect in the diff or its exact-SHA callers. The one Low finding concerns .. spellings saved as typed, and Low findings don't block approval.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 1m 32s
Token count: Unavailable

Pass 🔴 Critical 🟠 High 🟡 Medium ⚪ Low
🔒 Security 0 0 0 0
✅ Accuracy 0 0 0 1
🧰 Quality & Maintainability 0 0 0 0
Total 0 0 0 1

Key Findings

  1. ⚪ LOW — Accuracy: New `..` spellings are saved unnormalized, so on Unix they depend on the intermediate folder. crates/dashboard-core/src/projects.rs:368: Trigger: on Linux or macOS, a user links a new checkout as /src/sub/../hs-buddy. add_repository saves that exact text, which the new test a_new_checkout_is_saved_as_typed_and_found_again_under_another_spelling asserts. POSIX resolves sub/.. by walking sub, so if sub is later removed or renamed, the saved entry stops resolving and the row shows an identity error. The listed-entry canonicalize at line 361 then fails too, so linking the plain path adds a second row. This is inferred from path semantics and not observed at runtime. Issue #87's goal asks for 'one stable absolute path per checkout', but its acceptance criteria only forbid relative entries and the \\?\ prefix, so the impact is limited to this edge case. On Windows, Win32 removes .. lexically, so Windows is unaffected. (non-blocking)

Coverage

  • Files reviewed:
    • crates/dashboard-core/src/projects.rs
    • crates/dashboard-core/tests/projects.rs
    • crates/dashboard-app/src/captures.rs
  • Context read:
    • crates/dashboard-core/src/config.rs
    • crates/dashboard-core/src/pmd.rs
    • crates/dashboard-core/src/lib.rs
    • crates/dashboard-app/src/manage/projects.rs
    • crates/dashboard-app/src/projects_tests.rs
    • README.md
    • AGENTS.md
    • issue #87
    • PR #89 description
  • Verified:
    • Compare base_commit.sha and merge_base_commit.sha both equal b6b54508e0c21ee5bc3c269ae7f7ec0e0197b94b; the last commit is c7d344790c6c96b223fe46f694055990c155ab64; the branch is 2 ahead and 0 behind
    • is_absolute runs before any filesystem read, so a relative folder returns LinkError::Relative without loading or writing the config
    • canonicalize failure maps to LinkError::Folder; Display gives the not-found message for NotFound and the OS error otherwise; the error text contains no sensitive values
    • The resolved path is used only for comparison; add_repository still receives the typed checkout, so no \\?\ prefix reaches config.toml
    • The listed-entry comparison skips relative entries and entries that fail canonicalize, then returns the listed PathBuf; add_repository still re-reads the file and keeps its exact-match guard
    • App caller Manage::link_checkout runs the core call on the background executor and only displays the returned path; pushing a duplicate into linked is harmless because is_linked uses any
    • Capture helpers open_dialog, set, save, settle, click, name, Folder::checkout(&str,&str) and config_dir exist at the head SHA; dashboard_core::config is a public module
    • New core tests cover relative, ./, Windows \tickdown and C:tickdown, missing folder, exact path, trailing separator, .., Windows upper-case and Unix symlink spellings, and check the file stays byte-for-byte unchanged
    • README Link checkout text (line 220) does not contradict the new behavior, though it doesn't mention the full-path requirement
  • Not verified:
    • Compilation, clippy, and CI results on Ubuntu and Windows for this head: execution forbidden, and CI status was not read
    • Windows canonicalize case-folding and the C:tickdown and \tickdown handling were reasoned from std semantics, not observed
    • Whether open_dialog in the capture creates config.toml before the capture's add_repository call; that helper body was not read in full
    • macOS case-insensitive APFS behavior of canonicalize for case-only spellings
    • Latency of canonicalize on unreachable network-share entries (the PR states it as a known risk)
  • Tests run: None. Reviewer execution is disabled.

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head c7d344790c6c96b223fe46f694055990c155ab64 • SFL v0

@HemSoft
HemSoft merged commit af14360 into main Nov 6, 2026
5 checks passed
@HemSoft
HemSoft deleted the fix/issue-87-normalize-link-checkout-folder branch November 6, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model:claude-opus-5-5 PR authored by claude-opus-5-5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Normalize the Link checkout folder before checking and saving it

1 participant