Refuse to deploy a template onto its own source - #219
Conversation
If a directory is symlinked into place and a file inside that directory is also configured as a template, the template's target resolves through the directory symlink back to the source file. With --force, dotter removes the target - which is the source - and then fails reading it, destroying the file in the repository. Check that source and target are not the same file before deploying a template. Symlinks are unaffected: for those, target-is-source is the desired end state.
There was a problem hiding this comment.
Pull request overview
Adds a safety guard to prevent deploying a template onto the same underlying file as its source (e.g., when the target path resolves back into the repository through a symlink), addressing the destructive edge case in issue #218.
Changes:
- Add
check_not_self()and invoke it fromcreate_templateandupdate_templatebefore any template comparison/deploy logic. - Add a unit test covering “same file” detection via path aliasing and (on Unix) via a symlinked directory.
- Add
same-fileas a direct dependency to support robust same-file detection.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/actions.rs | Introduces and applies the self-overwrite guard and adds a unit test for same-file scenarios. |
| Cargo.toml | Adds same-file as a direct dependency. |
| Cargo.lock | Updates lockfile to reflect the new direct dependency edge. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| target.target | ||
| ); | ||
|
|
||
| check_not_self(source, &target.target)?; |
There was a problem hiding this comment.
All filesystem operations should go through the Filesystem trait, this facilitates testing. I think in this case we should implement this check inside one of the compare_templates, probably resulting in a new TemplateComparison variant
Then we can implement a test for the behavior in deploy.rs
|
|
||
| /// A template's target must never resolve to the template's own source. | ||
| fn check_not_self(source: &Path, target: &Path) -> Result<()> { | ||
| if same_file::is_same_file(source, target)? { |
There was a problem hiding this comment.
We might be able to use either the existing real_path function directly or at least fs::canonicalize to avoid adding a new dependency
Closes #218
This adds a check that a template's source and target are not the same file, on
create_templateandupdate_template. Symlinks are deliberately left alone: for those, target-is-source is the desired end state, and the check would fire on every correctly deployed link.same-fileis already inCargo.lockas a transitive dependency, so nothing new is compiled. Includes a unit test.there's prolly like nobody that ran into this tho =))