Dot file sync: support absolute host paths#43
Open
baxyz wants to merge 6 commits into
Open
Conversation
✅ PR Validation Passed
📋 Pipeline Status
🤖 Generated by @helpers4 CI • 2026-07-08 |
user.signingkey was rewritten from a host-absolute .ssh path to the container's TARGET_HOME/.ssh, but other bare-path keys (http.sslCert, http.sslKey, http.sslCAInfo) had the same class of bug and were copied verbatim. Generalize the rewrite to a small key allowlist, covering both .ssh/ and .gnupg/ targets. Also add a best-effort post-merge verification pass: for path-like keys that can't be deterministically rewritten (gpg.program, core.editor — host binary paths with no container equivalent), warn if the value doesn't resolve to an existing file instead of failing silently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Mirrors the REHOMEABLE_PATH_KEYS rewrite (including the .gnupg target and the non-allowlisted-key negative case) and the VERIFY_PATH_KEYS warning pass (missing vs. existing path) added in sync-files.sh. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…verify Code review of the previous commit found the generalized rewrite/verify mechanism was broken in several ways: - REHOMEABLE_PATH_KEYS used git's documented mixed-case spelling (http.sslCert/sslKey/sslCAInfo), but `git config --list` always lowercases key names — so 3 of the 4 allowlisted keys never matched and were silently never rewritten. Only user.signingkey worked, incidentally, because it's already all-lowercase. - The verify pass ran inside the .gitconfig merge block, before the .ssh/.gnupg syncs that actually copy the rehomed files into place — so a freshly-rewritten user.signingkey was flagged as "missing" on every first sync, training users to ignore the warning. - gpg.program/core.editor can hold "command --flags" strings, not bare paths; checking the full value for existence false-positived on any such value that happened to start with "/". - The .gnupg rewrite collapsed to basename only, losing subdirectory structure that the actual (recursive) .gnupg sync preserves. - credential.helper is flagged as host-specific in PROTECTED_KEYS but was missing from both new lists, despite commonly holding an absolute path to a custom helper script. Fixes: lowercase the allowlist to match git's normalization; move the verify pass to run after the .ssh/.gnupg syncs; strip a leading "!" and trailing arguments before checking path-like values; preserve the relative path (not just basename) when rewriting; add credential.helper as verify-only (not rewritten, since a blind rewrite would corrupt its "!cmd args" shell-invocation syntax); derive VERIFY_PATH_KEYS from REHOMEABLE_PATH_KEYS instead of hand-duplicating it, so the two can't drift apart again; and introduce a shared _key_in_list() helper used by PROTECTED_KEYS, REHOMEABLE_PATH_KEYS and VERIFY_PATH_KEYS instead of two different allowlist-membership idioms. Verified end-to-end against a synthetic host .gitconfig/.ssh/.gnupg fixture: mixed-case keys rewrite correctly, no false-positive warnings on a fresh sync, nested .gnupg paths preserve their subdirectory, and a missing credential.helper path is now caught. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous Test 5h called the rewrite helper directly with the
mixed-case key spelling ("http.sslKey"), so it never exercised the
`git config --list` normalization step that broke production (keys are
always lowercased) — it reported a false PASS while the equivalent real
flow silently never rewrote the value. Rewritten to build a real
git-config file and route it through `git config --file ... --list`,
same as sync-files.sh does, plus a nested .gnupg subdirectory case.
Test 5i now mirrors the corrected single-pass, post-sync verify loop:
covers the "!"-prefix and trailing-argument stripping (credential.helper,
gpg.program, core.editor) and confirms a bare-command core.editor value
doesn't false-positive.
Sanity-checked: reverting REHOMEABLE_PATH_KEYS to the old mixed-case
spelling makes the new Test 5h fail, confirming it's a real regression
guard for the bug it replaces coverage for.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Update the merge-strategy section to reflect the fixes: relative-path preservation for nested .gnupg values, verify running after the .ssh/.gnupg syncs, and credential.helper covered as verify-only. Note the git key-lowercasing behavior the allowlists depend on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please include a summary of what this PR does and why it's needed.
Type of Change
Related Issues
Closes #(issue number)
How Has This Been Tested?
Describe the tests you ran and how to reproduce them:
Checklist
Screenshots (if applicable)
Add screenshots for UI changes.
Additional Context
Add any other context about the PR here.