Resolve Merged-usr Paths Before Every dpkg-query -S Ownership Check

[ptr727]

Summary

tool_unshadow (host-setup/linux/install-tools.sh) asks dpkg-query -S "$resolved" whether a
shadowing file is distro-owned. On a merged-usr host, where /bin and /sbin are symlinks into
their /usr counterparts, a path spelled through the symlink (/bin/jq) matches nothing in
dpkg's database when a package built after that host's usrmerge records only the canonical
/usr/bin/jq spelling, so the ownership guard falls through and --upgrade removes a file dpkg
still believes it owns.

Adds a shared dpkg_owns_path helper and routes all three dpkg-query -S ownership call sites
in the file through it: tool_unshadow's own guard, ripgrep_download_path (an unowned rg
download reached through a merged-usr symlink would otherwise misreport as unowned and be
removed), and powershell_path_is_unowned, preserving each call site's existing exit-status
handling.

The helper checks the given path first, since a package that predates a host's usrmerge (common
on Debian bookworm and Ubuntu jammy/noble) keeps its file list recorded under the pre-merge
/bin or /sbin spelling, and only falls back to a directory-canonicalized spelling when that
misses. Only the directory is canonicalized, never the file name: an earlier version of this fix
canonicalized the whole path with realpath, which also follows a leaf symlink and made an
unowned symlink pointing at a dpkg-owned file (a hand-installed /usr/local/bin/jq -> /usr/bin/jq, say) misreport as owned. Two rounds of a local-strict-review pass caught that
regression plus the pre-merge-spelling gap before either reached this pull request; the second
round found no further defect.

Verification

• shellcheck and shfmt -d clean on the changed file (also run via scripts/docker_lint.py,
  along with cspell).
• bash -n syntax check clean.
• A constructed test (this host is itself merged-usr, so it reproduces the defect directly),
  extracting the actual dpkg_owns_path function from the shipped script and covering: the
  original #1866 symptom (/bin/ls recognized as owned via its canonicalized directory), the
  canonical path still recognized directly, a genuinely unowned scratch file still reporting
  unowned, a nonexistent path reporting unowned rather than erroring, an unowned symlink pointing
  at a dpkg-owned file not misreporting as owned (the first-round regression), a
  pre-merge-recorded path matched via a stubbed dpkg-query (the pre-merge-spelling gap), and a
  dpkg-query exit status of 2 propagating as 2 rather than collapsing to "unowned" -- all
  pass with the fix in place, and reverting to the original code fails the #1866 case as
  expected.
• Two independent adversarial local-strict-review passes (Opus tier) against the full diff, the
  first catching the two defects above and the second confirming the fix and finding nothing
  further.

Closes on promotion: #1866

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved package ownership checks for installed tools in systems with merged-usr paths, while preserving the distinction between unowned files and lookup errors.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 32f5d1d0-d047-4344-b0a8-b42eae348e74

📥 Commits

Reviewing files that changed from the base of the PR and between 20732b9 and 716ab9f.

📒 Files selected for processing (1)

• host-setup/linux/install-tools.sh

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

---

📝 Walkthrough

Walkthrough

Linux tool setup now uses a shared helper to check whether paths belong to installed packages. If the initial lookup reports “not found,” the helper retries with the directory canonicalized.

Changes

Linux package ownership checks

Layer / File(s)	Summary
Ownership lookup and tool integration host-setup/linux/install-tools.sh	The new dpkg_owns_path helper retries ownership lookup with the directory canonicalized when the initial lookup returns status 1. ripgrep_download_path, powershell_path_is_unowned, and tool_unshadow now use the helper and retain their existing handling of owned paths and lookup errors.

Priority: ⬇️ Low

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

Merge Risk: ⚪ Minimal · up to 716ab

The package-ownership fallback follows the documented merged-usr behavior, and no material merge-blocking risk was identified.

Architecture Summary

Architecture risk: 🟡 Medium · up to 716ab

The change affects 1 system.

Changed systems: host-setup

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — host-setup (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in host-setup/linux/install-tools.sh: Added dpkg_owns_path: it checks the supplied path first and, only when dpkg-query -S returns status 1, canonicalizes the directory and retries with the original leaf name. It returns the lookup’s status, returns 1 when there is no directory or canonicalization yields no changed directory, and does not canonicalize the leaf.
• observed — Modified behavior in host-setup/linux/install-tools.sh: ripgrep_download_path now uses dpkg_owns_path to identify package-owned resolved paths, including paths whose directory spelling differs from dpkg’s recorded canonical path; it still prints an unowned resolved path.
• observed — Modified behavior in host-setup/linux/install-tools.sh: powershell_path_is_unowned now uses dpkg_owns_path while retaining its status mapping: owned returns 1, not found returns 0, and other lookup errors terminate with an error.
• observed — Modified behavior in host-setup/linux/install-tools.sh: tool_unshadow now uses dpkg_owns_path before removing a PATH shadow, so a distro-owned file remains in place when dpkg records it under the canonical directory spelling.

Reliability and maintainability

• inferred — Risk-relevant change factors for host-setup: blast_radius_3; direct_dependents_3

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: resolving merged-usr paths for all dpkg-query -S ownership checks.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@20732b9). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff             @@
##             develop    #1951   +/-   ##
==========================================
  Coverage           ?   55.40%           
==========================================
  Files              ?       16           
  Lines              ?     7223           
  Branches           ?        0           
==========================================
  Hits               ?     4002           
  Misses             ?     3221           
  Partials           ?        0           

Flag	Coverage Δ
python-3.13	55.40% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Copilot review overview

🟢 Approval recommended

The change is narrowly scoped, preserves existing exit-status handling at each call site, and correctly addresses merged-usr ownership lookups without following leaf symlinks.

Review effort: Lite
Findings: None

What changed in this PR

This PR fixes dpkg ownership checks on merged-usr Linux hosts by ensuring dpkg-query -S is asked about a directory-canonicalized spelling (e.g., /usr/bin/...) when the original lookup via a symlinked directory (e.g., /bin/...) returns “not found”, preventing --upgrade from removing dpkg-owned files.

Changes:

• Add dpkg_owns_path helper that checks ownership for the given path first, then retries with the directory canonicalized (preserving dpkg-query exit-status semantics).
• Route the three existing dpkg-query -S ownership checks (tool_unshadow, ripgrep_download_path, powershell_path_is_unowned) through the helper.

File	Description
host-setup/​linux/​install-tools.sh	Introduces dpkg_owns_path and updates dpkg ownership call sites to handle merged-usr path spellings safely.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ptr727]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.