feat(export): add --to-files to write per-path .env files#258
Conversation
Add a --to-files flag to `infisical export` that writes one output file per logical secret path, mirroring the folder path into the filesystem (e.g. apps/cli/.env). `--path="apps/*"` targets the immediate subfolders of a folder; omitting --path exports every folder in the tree. Reuses the existing formatters and folder-listing API. --to-files is mutually exclusive with --output-file. Implements Infisical/infisical#1068 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| packages/cmd/export_to_files.go | New file implementing --to-files logic; has a path traversal bug (server-controlled folder names written directly to disk), no cycle detection in recursive tree walk, and a redundant MkdirAll call. |
| packages/cmd/export.go | Adds --to-files flag registration and dispatch; mutual-exclusion check with --output-file is correct and integration with existing flag parsing is straightforward. |
| packages/cmd/export_to_files_test.go | Table-driven unit tests for path expansion and file mapping helpers; covers all documented cases but does not exercise path traversal or cyclic-tree inputs. |
Reviews (1): Last reviewed commit: "feat(export): add --to-files to write pe..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| paths := make([]string, 0, len(folders)) | ||
| for _, folder := range folders { | ||
| paths = append(paths, base+"/"+folder.Name) | ||
| } |
There was a problem hiding this comment.
folder.Name comes directly from the Infisical API and is concatenated into a path without any sanitization. filepath.Join resolves .. components, so a server returning a folder named .. or ../../etc would cause the CLI to write secret files outside the current working directory. For example, if the API returns Name: "../../.ssh", the resulting output path becomes ../../.ssh/.env, silently overwriting files in unintended locations.
Consider validating that folder.Name contains no path separators or .. components before appending it, e.g. using filepath.Base(folder.Name) to strip any directory components or explicitly rejecting names that contain / or ...
| func walkFolderTree(root string, listChildren func(parent string) ([]string, error)) ([]string, error) { | ||
| paths := []string{root} | ||
| children, err := listChildren(root) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| for _, child := range children { | ||
| descendants, err := walkFolderTree(child, listChildren) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| paths = append(paths, descendants...) | ||
| } | ||
| return paths, nil | ||
| } |
There was a problem hiding this comment.
walkFolderTree
If the Infisical API returns a folder structure where a child path equals or prefixes an ancestor (e.g. listChildren("/apps") returns ["/apps"]), walkFolderTree recurses infinitely and will crash with a stack overflow. This could be triggered by a buggy or adversarial server response. A simple visited-set check on the path before recursing would prevent this.
| outputFile := mapPathToFile(path, format) | ||
| if dir := filepath.Dir(outputFile); dir != "." { | ||
| if err := os.MkdirAll(dir, 0755); err != nil { | ||
| return fmt.Errorf("failed to create directory %s: %w", dir, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
os.MkdirAll call
writeToFile (defined in ssh.go) already calls os.MkdirAll on the directory before writing. The explicit os.MkdirAll here is never needed and can be removed to avoid the duplicated logic.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| func expandToFilePaths(pattern string, listChildren func(parent string) ([]string, error)) ([]string, error) { | ||
| if strings.HasSuffix(strings.TrimSpace(pattern), "/*") { | ||
| parent := normalizeSecretPath(strings.TrimSuffix(strings.TrimSpace(pattern), "/*")) | ||
| return listChildren(parent) | ||
| } | ||
|
|
||
| path := normalizeSecretPath(pattern) | ||
| if path == "/" { | ||
| return walkFolderTree("/", listChildren) | ||
| } | ||
| return []string{path}, nil |
There was a problem hiding this comment.
* anywhere but the end are silently treated as concrete paths
Only the suffix /* is detected as a glob. A user supplying apps/*/web or */cli will not get an error — the pattern is passed through normalizeSecretPath and treated as a literal Infisical path, almost certainly returning zero secrets with no diagnostic message. Consider returning an error for patterns that contain * in a position other than the trailing /*.
Validate that server-returned folder names are safe path segments before mirroring them into the filesystem (reject "..", ".", and path separators) so a malicious or buggy API response cannot write secret files outside the target directory. Add cycle detection to the recursive folder walk to prevent infinite recursion on a cyclic folder graph. Drop the redundant MkdirAll since writeToFile already creates parent directories. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the thorough review. Addressed both P1s and the redundant-MkdirAll P2 in f41ab80:
On the P2 about a mid-pattern |
1 participant
Description
Adds a
--to-filesflag toinfisical export. Closes Infisical/infisical#1068.Today
exportwrites a single stream (stdout, or one file via--output-file). In a monorepo you have to run a separateinfisical export --path="apps/cli" > apps/cli/.envfor every app and package to reflect the logical secret-folder layout into the physical filesystem.--to-filesdoes that in one command, mirroring each logical path into the filesystem:infisical export --path="apps/*" --to-fileswritesapps/cli/.env,apps/api/.env,apps/web/.env(one file per immediate subfolder ofapps)--pathexports every folder in the tree, one file per path--path="/apps/cli"writes the singleapps/cli/.envThe output filename follows
--format(.env,secrets.json,secrets.yaml, ...). It reuses the existing formatters,util.GetAllFolders, and the per-path secret fetch, and is mutually exclusive with--output-file.Type of change
How has this been tested?
The path-resolution logic is split into pure helpers (
expandToFilePaths,mapPathToFile) so it can be unit-tested without a live backend.packages/cmd/export_to_files_test.go(testify table tests) covers: glob to immediate children only, root-glob to top-level (non-recursive), omitted/root to a full recursive walk, concrete path to itself, leading/trailing-slash normalization, and the per-format filename mapping (12 cases).go build ./...passesgo test -vet=off -run 'TestExpandToFilePaths|TestMapPathToFile' ./packages/cmd/passes (-vet=offonly to skip a pre-existinggo vetfinding inrun.go, unrelated to this change)infisical export --helpshows the new flagThe end-to-end fetch-and-write against a real project needs a live Infisical backend (the
test/integration suite uses real credentials), so the resolution/mapping logic lives behind injectable helpers that are unit-tested directly.One thing worth a maintainer's eye: I read "if the option is not provided, it will create a .env for all the paths" as a full recursive walk, and
apps/*as a single level of children. Happy to adjust the glob/recursion semantics if you'd prefer different behavior (e.g. an explicit--recursive).Checklist