fix(generate): use relative paths for env vars in generated README#2865
Open
mikeland73 wants to merge 3 commits into
Open
fix(generate): use relative paths for env vars in generated README#2865mikeland73 wants to merge 3 commits into
mikeland73 wants to merge 3 commits into
Conversation
devbox generate readme rendered plugin-provided environment variables (e.g. PGDATA, PGHOST from the postgresql plugin) with their fully expanded absolute paths, leaking machine-specific paths such as the user's home directory into the committed README. Replace the absolute project directory with "." in env var values, mirroring the existing Scripts.WithRelativePaths behavior already applied to scripts in the generated README. Fixes #2178
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes devbox generate readme so that plugin-provided environment variables rendered into the generated README use project-relative paths instead of fully expanded absolute paths, avoiding leakage of machine-specific directories/usernames.
Changes:
- Apply project-dir-to-
.substitution to generated README env var values (mirroring existing scripts behavior). - Add focused unit tests for env var relative-path handling (substitution, non-mutation, empty project dir behavior).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/devbox/docgen/docgen.go | Adds env var path relativization before README template rendering. |
| internal/devbox/docgen/docgen_test.go | Adds tests covering the new env var relative-path behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…st data Address review feedback: - envWithRelativePaths now returns an independent copy even when projectDir is empty (via maps.Clone), matching its doc comment so callers can always mutate the result safely. - Use a generic project path in the test fixture instead of a real-looking username.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2178.
devbox generate readmerendered plugin-provided environment variables with their fully expanded absolute paths. For example, enabling thepostgresqlplugin produced a README containing:This leaks machine-specific paths — including the user's home directory / username — into a file that is typically committed to the repository.
The generated README already strips the absolute project directory out of scripts via
configfile.Scripts.WithRelativePaths(projectDir). Environment variables were the one place this wasn't applied, becauseConfig().Env()expands included-plugin env values to absolute paths before they reach the template.Fix
Apply the same relative-path treatment to env var values before rendering: replace the absolute project directory with
., so the example above becomes:This is intentionally consistent with the existing
Scripts.WithRelativePathsbehavior (samestrings.ReplaceAll(value, projectDir, ".")transformation), and matches what was requested on the issue:Values that don't contain the project dir (e.g.
PGPORT="5432") are left untouched.How was it tested?
internal/devbox/docgen/docgen_test.gocovering: project-dir substitution, that the input map is not mutated, and that an empty project dir returns env unchanged.go test ./internal/devbox/docgen/passes;go vetandgofmtare clean.cc the original reporter (the issue's author account has since been deleted) and @ametad, who reconfirmed the request on the issue.
Generated by Claude Code