fix(scaffold): resolve relative write-target directories consistently

[Erik Osterman (Cloud Posse) (osterman)]

what

• Fixes atmos scaffold generate so a relative target directory (e.g. the CLI's own default ./my-project) works, instead of rejecting every file with path traversal not allowed.
• validateWriteTarget in pkg/generator/engine/templating.go now resolves the write directory (realDir) through the same ResolveAndCleanBasePath helper already used for the target base (realBase), instead of a bare filepath.EvalSymlinks that stays relative for relative inputs.
• Adds a regression test, TestProcessFile_RelativeTargetPath, covering a relative targetPath end-to-end (previous tests only exercised absolute t.TempDir() targets, so this case was never caught).

why

• realBase was always absolutized before comparison, but realDir was resolved with a bare filepath.EvalSymlinks, which returns a relative path unchanged when given a relative input. Comparing an absolute path against a relative one never matched the containment check, so it fired as a false-positive path traversal on every write whenever the target directory was relative — including the command's own default target.
• Absolute targets happened to work only because filepath.Dir(fullPath) was already absolute in that case, masking the bug.

references

• Closes atmos scaffold generate fails with "path traversal not allowed" for every file when the target is a relative path #2851

Summary by CodeRabbit

• Bug Fixes

  • Fixed file generation for relative target paths, such as ./my-project.
  • Improved path resolution while preserving containment and symlink safety checks.

• Tests

  • Added coverage confirming generated files are written to the expected destination with the correct content.

[atmos-pro]

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

[coderabbitai]

📝 Walkthrough

Walkthrough

validateWriteTarget now resolves the containing directory with ResolveAndCleanBasePath. A regression test verifies that ProcessFile writes expected content to a relative target directory.

Changes

Relative write-target handling

Layer / File(s)	Summary
Resolve relative write targets pkg/generator/engine/templating.go, pkg/generator/engine/templating_test.go	validateWriteTarget uses shared base-path resolution. TestProcessFile_RelativeTargetPath verifies file creation, destination, and content for a relative target.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested reviewers: aknysh, goruha, johncblandii

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the fix for consistent relative write-target resolution.
Linked Issues check	✅ Passed	The code fixes relative target validation and adds regression coverage for relative paths, satisfying issue #2851.
Out of Scope Changes check	✅ Passed	All changes directly support the relative write-target fix and its regression test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch osterman/fix-issue-2851

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.10%. Comparing base (f777b18) to head (b7205c5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2855   +/-   ##
=======================================
  Coverage   82.09%   82.10%           
=======================================
  Files        1802     1802           
  Lines      175133   175133           
=======================================
+ Hits       143779   143786    +7     
+ Misses      23589    23581    -8     
- Partials     7765     7766    +1     

Flag	Coverage Δ
unittests	82.10% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/generator/engine/templating.go	95.80% <100.00%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Erik Osterman (Cloud Posse) (osterman)]

CodeRabbit (@coderabbitai) full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

🧹 Nitpick comments (1)

pkg/generator/engine/templating_test.go (1)

644-676: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add a relative-target escape regression case.

The test proves that ./my-project succeeds. It does not prove that a relative target still rejects a symlinked directory outside the target. Add a case that asserts errors.Is(err, errUtils.ErrPathTraversal) and confirms that no file appears in the outside directory.

As per coding guidelines, new Go features require comprehensive unit tests and negative-path coverage.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/generator/engine/templating_test.go` around lines 644 - 676, Extend
TestProcessFile_RelativeTargetPath with a negative case where a directory inside
the relative target points via symlink outside the target; assert ProcessFile
returns an error matching errUtils.ErrPathTraversal and verify the expected file
is not created in the outside directory. Preserve the existing successful
relative-target assertions.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/generator/engine/templating_test.go`:
- Around line 644-676: Extend TestProcessFile_RelativeTargetPath with a negative
case where a directory inside the relative target points via symlink outside the
target; assert ProcessFile returns an error matching errUtils.ErrPathTraversal
and verify the expected file is not created in the outside directory. Preserve
the existing successful relative-target assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c9ac448-8679-4775-a88c-d755ec504941

📥 Commits

Reviewing files that changed from the base of the PR and between f777b18 and b7205c5.

📒 Files selected for processing (2)

• pkg/generator/engine/templating.go
• pkg/generator/engine/templating_test.go

[atmos-pro]

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

[github-actions]

These changes were released in v1.225.0-rc.6.