Rodin
aee0927cfb
CI / test (pull_request) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 31s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 56s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m17s
The previous implementation called os.Lstat(absPath) which only avoids following the *final* path component. A PR committing .review-bot/ as a directory symlink pointing outside the repo would pass the filepath.Rel confinement check because the textual path was inside the root while the resolved destination was not. Fix: call filepath.EvalSymlinks after filepath.Abs to resolve ALL symlink components before the confinement check. If EvalSymlinks fails (dangling symlink, nonexistent target) the path is rejected. The filepath.Rel check then operates on the fully-resolved path. Semantic change: file-level in-repo symlinks (target also within root) are now allowed — the invariant is about where the content lives, not whether the entry is a symlink. The test TestValidateDocmapPath_Symlink is updated to test an out-of-repo symlink target, which must still be rejected. Tests: - TestValidateDocmapPath_DirSymlinkBypass: reproduces the attack vector (dir symlink bypassing textual confinement check) and verifies it is now rejected - TestMainSubprocess_InvalidDocMapPath: subprocess test via validate-docmap subcommand — ../../../etc/passwd rejected (closes #146) - TestMainSubprocess_InvalidDocMapFile: subprocess test — nonexistent file rejected (closes #146) Coverage: 54.3% (was 54.1%)
50 lines
3.2 KiB
Markdown
50 lines
3.2 KiB
Markdown
# CHANGELOG
|
|
|
|
## Unreleased
|
|
|
|
### Security
|
|
|
|
- **`validateDocmapPath`: add `EvalSymlinks` to close directory-symlink bypass** ([#150](https://gitea.weiker.me/rodin/review-bot/issues/150)): The previous implementation used `os.Lstat` which only avoids following the *final* path component. An intermediate directory symlink (e.g. `.review-bot/` committed as a symlink to a directory outside the repo) would pass the path-confinement check because the textual path appeared within the repo root. `filepath.EvalSymlinks` is now called first, resolving all symlink components before the `filepath.Rel` confinement check. In-repo symlinks whose resolved targets also reside within the repo root are now allowed; out-of-repo targets are rejected by the confinement check.
|
|
|
|
### Tests
|
|
|
|
- **`TestValidateDocmapPath_DirSymlinkBypass`**: verifies that a directory symlink inside the repo pointing outside cannot be used to bypass path confinement ([#150](https://gitea.weiker.me/rodin/review-bot/issues/150)).
|
|
- **`TestMainSubprocess_InvalidDocMapPath`**: verifies that a path-traversal `--docmap` argument (`../../../etc/passwd`) is rejected by the validate-docmap subcommand ([#146](https://gitea.weiker.me/rodin/review-bot/issues/146)).
|
|
- **`TestMainSubprocess_InvalidDocMapFile`**: verifies that a nonexistent `--docmap` file is rejected by the validate-docmap subcommand ([#146](https://gitea.weiker.me/rodin/review-bot/issues/146)).
|
|
|
|
### Added
|
|
|
|
- **`doc-map` input** (`--doc-map` flag / `DOC_MAP_FILE` env var): Path to a YAML file mapping source path globs to governing design docs. review-bot intersects the map with changed PR paths and injects matching docs into the system prompt under a `## Design Documents` heading. ([#137](https://gitea.weiker.me/rodin/review-bot/issues/137))
|
|
- **`doc-map-max-bytes` input** (`--doc-map-max-bytes` flag / `DOC_MAP_MAX_BYTES` env var): Cap on total injected design doc content in bytes. Default: 102400 (100 KB). Prevents accidental context overflow when a PR touches many modules.
|
|
- **`DesignDocs` budget section**: Design docs are included in the context budget and trimmed after conventions, before file context, if the total exceeds the model's context limit.
|
|
|
|
### Doc-map config format
|
|
|
|
```yaml
|
|
mappings:
|
|
- paths:
|
|
- "lib/gargoyle/engine/signal_risk/**"
|
|
docs:
|
|
- docs/domain/contexts/risk/risk-controls.md
|
|
- paths:
|
|
- "lib/gargoyle/trading/**"
|
|
docs:
|
|
- docs/domain/contexts/trading/
|
|
```
|
|
|
|
- `paths` — glob patterns (including `**`) matched against changed file paths in the PR
|
|
- `docs` — local file paths or directories (all `.md` files under a directory) to inject
|
|
- Multiple mappings can reference the same doc; docs are deduplicated
|
|
- Missing doc files: warn and skip (review continues without them)
|
|
- No matching paths: no docs injected, review runs normally
|
|
- Absolute paths and path traversal (`..` segments) in doc paths are rejected
|
|
|
|
### Security
|
|
|
|
- **Path traversal guard**: doc paths from the YAML config are validated to reject absolute paths and `..` segments before VCS API calls
|
|
- **Prompt injection guard**: design doc content is injected with an explicit instruction to treat it as reference data and not follow any instructions it may contain
|
|
|
|
## v0.3.2
|
|
|
|
- Previous releases tracked in Gitea release notes.
|