fix(perms): never dereference symlinks when applying the ownership table

[thinmintdev]

What

hal0 doctor perms --fix (and every install/upgrade, which runs it) dereferenced symlinks and rewrote the files they point at. Fixes #1739 (P0, regression from #1732).

hal0 migrate model-layout --apply plants /var/lib/hal0/models/<name> → /mnt/ai-models/<name> symlinks. The perms table's recursive models/ row matched them via rglob, and commit() → _apply_one() called os.chown/os.chmod, both of which follow symlinks (Linux has no lchmod). Result: the operator's real model-store files were re-owned hal0:hal0 and re-moded 0644, outside hal0's declared tree, on every install and every upgrade — directly contradicting migrate_commands.py's documented "does not write to /mnt/ai-models at all" invariant.

How

The whole commit path is now lstat-based and symlink-skipping, at three layers:

1. _expand_row drops any glob match that is a symlink or that was reached by walking through one. The second case matters: migrate model-layout links whole per-model directories, so recursing into the link yields real paths under /mnt/ai-models that are not themselves symlinks — a link check in _apply_one alone would not save them. Checked explicitly rather than relying on Path.rglob's no-follow behaviour, so the guarantee does not depend on the pathlib version.
2. PermObservation.is_symlink (observations were already lstat-based) + PermDiff.changed is never true for a symlink. A declared row whose own target is a link plans as a no-op and audits as a new symlink status instead of drift (badge added to doctor_commands._render_audit; _diagnose_audit_rows carries no finding for it, like absent).
3. _apply_one hard-refuses a symlink — last-line guard covering the rollback path and any future caller.

Skipping (rather than chown(follow_symlinks=False) on the link itself) is the safer choice and matches migrate's invariant: a link target's ownership is not hal0's concern. The skip is surgical — real files under the same row are still reconciled.

install.sh ordering comment

#1732 deliberately placed the migration before doctor perms --fix so planted symlinks would be "swept into that recursive re-chown". That intent is the bug. With this fix symlinks are skipped, so the ordering no longer matters; the comment is corrected to say so. The migration itself is not reverted — it correctly fixes the #1615 doctor warning.

Verification

Red-first. The five new tests in tests/install/test_perms.py (which had zero symlink coverage) all fail against the pre-fix code — the review's exact repro, a symlink under a recursive PermRow, shows the target's mode flipping 0o600 → 0o644:

FAILED test_commit_never_dereferences_symlink_under_recursive_row
FAILED test_plan_skips_paths_reached_through_a_symlinked_directory
FAILED test_non_glob_symlink_row_is_never_drift_and_audits_as_symlink
FAILED test_apply_one_hard_refuses_a_symlink_even_when_planned
FAILED test_regular_files_are_still_reconciled_alongside_symlinks
5 failed, 27 deselected

With the fix:

• tests/install tests/installer — 570 passed, 1 skipped (shellcheck not installed locally)
• + tests/security tests/agents/test_hermes_security_deliverables.py — 763 passed
• tests/cli (covers the _render_audit badge change) — 677 passed
• ruff check clean, ruff format --check clean
• mypy on both touched files: only the pre-existing doctor_commands.py:386 no-any-return, outside this diff

Risk

Strictly safer: this only stops writes to paths hal0 never should have touched. No path that was correctly reconciled before is skipped now (covered by test_regular_files_are_still_reconciled_alongside_symlinks).

Closes #1739

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.