[shiqui]

Description

A config symlink whose target filename ends in newlines could be validated against the truncated filename. This could accept an unsafe writable target or reject a safe target because the truncated path is writable.

Use readlink -n with a sentinel to preserve the exact target, including multiple trailing newlines, while keeping read failures fatal. Suppressing the output delimiter avoids relying on differing macOS and GNU readlink newline behavior.

Motivation

Fix major bug-scan finding #3 in the config-safety validator introduced by #225, now merged into main.

Type of Change

• Bug fix

Testing

Tested on macOS with Bash 3.2.57 and Git 2.50.1, using disposable repositories and isolated Git configuration/XDG paths.

• All 25 config-safety BATS tests pass. Four new regressions cover unsafe absolute and relative newline-terminated targets, a safe target with a writable truncated-path counterpart, and readlink failing after printing a safe path.
• ShellCheck passes for all CI-targeted scripts and the modified test file.
• Completion generation and git diff --check pass.
• Bash 3.2 CLI smoke passes for config set/get, worktree creation, listing, removal, and rejection of an unsafe newline-terminated target.
• Full suite: 729 passed, 7 failed. All seven failures reproduce on unchanged baseline 18f0914: three PR remote-selection tests, three debug-trap tests, and the macOS terminal-quoting test. One PR test attempts a GitHub fetch unavailable in the test environment.

The branch was rebased onto merged #225; the resulting source tree is identical to the validated tree. Linux and Windows were not tested locally.

Breaking Changes

None beyond enforcing the existing config-safety policy on the actual symlink target.

Checklist

• Followed project style and Bash 3.2 compatibility.
• Added focused regression coverage and performed manual testing on macOS.
• Introduced no new dependencies, commands, flags, or configuration keys.
• Documented existing test failures and platform validation limits.

License Acknowledgment

This contribution is made under the Apache License 2.0.

Summary by CodeRabbit

• Bug Fixes
  • Corrected handling of symlink targets whose filenames end with newline characters, so safety checks evaluate the full target path.
  • Unsafe targets are blocked, while protected targets with trailing newlines can be used as expected.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The config-safety validator now preserves trailing newlines when it reads symlink targets with readlink -n and a sentinel. Regression tests cover writable and protected targets, plus a read failure after output.

Changes

Config Safety

Layer / File(s)	Summary
Exact symlink target validation lib/config_safety.sh, tests/config_safety.bats	The validator preserves trailing newlines in symlink targets. Tests verify rejection of writable absolute and relative targets, acceptance of a protected target despite a writable truncated-path counterpart, and rejection when readlink exits unsuccessfully.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: natoboram

Merge Risk: Minimal · up to 91c3a

Newline-terminated symlink targets are preserved, and failed reads block validation. No merge-blocking risk was identified.

Security architecture risk: Low · up to 91c3a

The fix validates the actual symlink target and preserves blocking behavior when reading it fails. No new execution bypass was identified. Existing generated shell helpers can retain earlier validation code when their cache stamp still matches; upgrade behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The relevant attacker capability is modifying or replacing a configuration target or its path through filesystem permissions. If unsafe configuration is accepted, configured commands execute with the invoking process's authority, and configuration can influence worktree destinations. The inspected change does not add a remote, tenant, or cross-service entrypoint.

Security Findings and Attack Paths

• inferred — No introduced or worsened security condition was established. The changed path closes a mismatch between the symlink target consumed and the pathname validated. Repository-local hook execution remains gated; the supplied security assessment retained no findings.

Trust Boundaries and Controls

• observed — A readlink failure cannot become successful validation merely by printing a protected path: the sentinel is conditional on success, failure returns through the validator and retrieval gate, and neither hook executor reaches eval. The new failure regression also asserts no hook marker, destination, or branch.

Hardening Proposals

• proposed — For same-version upgrades, explicitly invalidate or regenerate shell-integration caches so embedded validators adopt the correction. This would address the pre-existing stale-code mechanism, not an observed new execution bypass.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preserving trailing newlines in config symlink targets.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the link path true,
No trailing newline slips from view.
Unsafe targets meet a stop,
Safe ones pass the careful hop.
Tests watch each newline's flight,
And guard the hook from false green light.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: coderabbitai/git-worktree-runner
• Branch: main
• File: .coderabbit.yaml
• Commit: ce66bb3
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 07d7c2e2-b544-4419-8f72-5c6babf278ea

Commits

Reviewing files that changed from the base of the PR and between ce66bb3 and 91c3a69.

Files selected for processing: 2

• lib/config_safety.sh
• tests/config_safety.bats

Included review availability: This review used your included allowance. Your plan provides up to 100 included reviews per hour; 96 remain after this review.