Skip to content

post-validate.sh: path handling edge cases + missing test coverage #74

Description

@5uck1ess

Follow-up from the #72 review. The portable `realpath -m` replacement fixes the macOS false-positive but has three edge cases that weren't fully addressed. Grouping them here since the fix + tests cluster naturally.

Edge cases

1. Unnormalized `..` escape when cwd == repo root

`post-validate.sh:61-66` does:
```bash
case "$FILE_PATH" in
/*) ABS_PATH="$FILE_PATH" ;;
) ABS_PATH="$(pwd)/$FILE_PATH" ;;
esac
```
When cwd == `$REPO_ROOT` and `FILE_PATH=../sibling.go`, `ABS_PATH` becomes `/path/to/repo/../sibling.go`, which still matches the `"$REPO_ROOT"/
` glob — silently passing the check even though the resolved path is outside the repo.

The comment in the source says "..-escapes will (correctly) not match" — that's only true when cwd is a subdirectory of the repo, not the repo root itself.

Fix options:

  • Add `case "$ABS_PATH" in ..) emit warning ;; esac` guard (cheap)
  • Normalize via `python3 -c 'import os,sys; print(os.path.abspath(sys.argv[1]))' "$FILE_PATH"` (portable, slower)
  • Use a shell-level loop to strip `..` segments (portable, finicky)

2. Symlinked repo roots

If the user's repo root is reached via a symlink (e.g. `/Users/x/dev` → `/Volumes/Work/dev`) and `$(pwd)` follows a different resolution than `git rev-parse --show-toplevel`, `ABS_PATH` and `REPO_ROOT` can have mismatching prefixes and in-repo writes get flagged "outside repo." False-positive warning, not a security issue, but it's a new misclassification that didn't exist on GNU systems before the realpath fix.

Fix: normalize `REPO_ROOT` through the same shell-level path:
```bash
REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || true)
[ -n "$REPO_ROOT" ] && REPO_ROOT=$(cd "$REPO_ROOT" && pwd)
```

Missing test coverage

The existing tests in #72 run from `$REPO_ROOT` with a plain relative path and an absolute `/opt/…` path. They don't exercise:

Source

  • Reviewed by `pr-review-toolkit:code-reviewer` (confidence 80)
  • Reviewed by `pr-review-toolkit:pr-test-analyzer` (criticality 5-6)
  • Reviewed by `pr-review-toolkit:silent-failure-hunter` (LOW severity)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions