Merge pull request 'feat(#141): validate-docmap subcommand' (#156) from issue-141 into main
CI / test (push) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / test (push) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
Reviewed-on: #156 Reviewed-by: security-review-bot <10+security-review-bot@noreply.gitea.weiker.me>
This commit was merged in pull request #156.
This commit is contained in:
@@ -61,6 +61,13 @@ func validateDocmapPath(localPath, resolvedRoot string) error {
|
|||||||
return fmt.Errorf("symlinks are not allowed")
|
return fmt.Errorf("symlinks are not allowed")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reject anything that is not a regular file (directories, FIFOs, device
|
||||||
|
// nodes, etc.) — ParseDocMapConfig expects a plain YAML file and would
|
||||||
|
// produce a confusing error on non-regular entries.
|
||||||
|
if !fi.Mode().IsRegular() {
|
||||||
|
return fmt.Errorf("docmap must be a regular file")
|
||||||
|
}
|
||||||
|
|
||||||
// Confine to resolvedRoot: use the fully-resolved path so that a directory
|
// Confine to resolvedRoot: use the fully-resolved path so that a directory
|
||||||
// symlink inside the repo cannot carry the path outside the root.
|
// symlink inside the repo cannot carry the path outside the root.
|
||||||
rel, err := filepath.Rel(resolvedRoot, resolvedPath)
|
rel, err := filepath.Rel(resolvedRoot, resolvedPath)
|
||||||
@@ -171,6 +178,9 @@ func runValidateDocmap(args []string) int {
|
|||||||
// Normalize Windows-style backslashes to forward slashes so that
|
// Normalize Windows-style backslashes to forward slashes so that
|
||||||
// changed-file paths from git on Windows match doc-map globs.
|
// changed-file paths from git on Windows match doc-map globs.
|
||||||
f = strings.ReplaceAll(f, "\\", "/")
|
f = strings.ReplaceAll(f, "\\", "/")
|
||||||
|
// Strip a leading "./" emitted by non-git tools (e.g. `find`) so that
|
||||||
|
// paths like "./cmd/foo.go" match doc-map globs written as "cmd/**".
|
||||||
|
f = strings.TrimPrefix(f, "./")
|
||||||
if !review.FileCoveredByDocMap(cfg, f) {
|
if !review.FileCoveredByDocMap(cfg, f) {
|
||||||
uncovered = append(uncovered, f)
|
uncovered = append(uncovered, f)
|
||||||
}
|
}
|
||||||
@@ -189,7 +199,7 @@ func runValidateDocmap(args []string) int {
|
|||||||
staleDocs := checkStaleDocs(cfg, resolvedRoot)
|
staleDocs := checkStaleDocs(cfg, resolvedRoot)
|
||||||
if len(staleDocs) > 0 {
|
if len(staleDocs) > 0 {
|
||||||
failed = true
|
failed = true
|
||||||
fmt.Fprintln(errWriter, "ERROR: stale docmap docs: entries (paths do not exist):")
|
fmt.Fprintln(errWriter, "ERROR: stale docmap entries (paths do not exist):")
|
||||||
for _, d := range staleDocs {
|
for _, d := range staleDocs {
|
||||||
fmt.Fprintf(errWriter, " %s\n", d)
|
fmt.Fprintf(errWriter, " %s\n", d)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -599,3 +599,53 @@ func TestValidateDocmapPath_DirSymlinkBypass(t *testing.T) {
|
|||||||
t.Error("expected rejection of dir-symlink bypass, got nil error")
|
t.Error("expected rejection of dir-symlink bypass, got nil error")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestValidateDocmapPath_NonRegularFile verifies that --docmap pointing at a
|
||||||
|
// non-regular file (e.g. a directory) is rejected with a clear error before
|
||||||
|
// ParseDocMapConfig is called.
|
||||||
|
func TestValidateDocmapPath_NonRegularFile(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
// Use the directory itself as the docmap path — directories pass Lstat but
|
||||||
|
// are not regular files.
|
||||||
|
reviewBotDir := filepath.Join(dir, ".review-bot")
|
||||||
|
if err := os.MkdirAll(reviewBotDir, 0o755); err != nil {
|
||||||
|
t.Fatalf("MkdirAll: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
code, _, stderr := stdinValidateDocmap(t,
|
||||||
|
"",
|
||||||
|
[]string{"--docmap", reviewBotDir, "--repo-root", dir},
|
||||||
|
)
|
||||||
|
if code != 2 {
|
||||||
|
t.Errorf("expected exit 2 for directory docmap, got %d; stderr: %q", code, stderr)
|
||||||
|
}
|
||||||
|
if !strings.Contains(stderr, "regular file") && !strings.Contains(stderr, "invalid") {
|
||||||
|
t.Errorf("expected regular-file rejection in stderr, got %q", stderr)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRunValidateDocmap_DotSlashPrefix verifies that paths emitted with a
|
||||||
|
// leading "./" (e.g. from `find` or `ls`) match doc-map globs correctly.
|
||||||
|
// Without TrimPrefix, "./cmd/foo.go" would not match the pattern "cmd/**".
|
||||||
|
func TestRunValidateDocmap_DotSlashPrefix(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
makeDocFile(t, dir, "docs/foo.md")
|
||||||
|
|
||||||
|
docmap := makeDocmapInDir(t, dir, `
|
||||||
|
mappings:
|
||||||
|
- paths:
|
||||||
|
- "cmd/**"
|
||||||
|
docs:
|
||||||
|
- docs/foo.md
|
||||||
|
`)
|
||||||
|
|
||||||
|
// File with a leading "./" should be treated as covered.
|
||||||
|
code, _, stderr := stdinValidateDocmap(t,
|
||||||
|
"./cmd/foo.go\n",
|
||||||
|
[]string{"--docmap", docmap, "--repo-root", dir},
|
||||||
|
)
|
||||||
|
if code != 0 {
|
||||||
|
t.Errorf("expected exit 0 for './' prefixed covered file, got %d; stderr: %q", code, stderr)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user