Preserve PVE authorized_keys symlinks in setup scripts

Refs #1297
This commit is contained in:
rcourtman 2026-05-01 18:38:09 +01:00
parent e66fd1b976
commit 67f2ce5dc2
8 changed files with 243 additions and 15 deletions

View file

@ -0,0 +1,45 @@
# Known RC Issue Closure For GA Authorized Keys Symlink Record
- Date: `2026-05-01`
- Gate: `known-rc-issue-closure-for-ga`
- Result: `passed`
## Context
The v5 maintenance delta audit found that `#1297` had only been partially
carried into v6. The v6 Proxmox setup script already escaped the temperature
monitoring forced-command key entry, but its install and uninstall paths still
rewrote `/root/.ssh/authorized_keys` directly.
That preserved the old regression class on Proxmox hosts where
`/root/.ssh/authorized_keys` is a symlink into `/etc/pve/priv/`: filtering old
Pulse keys through a local temp file and moving it back to the symlink path can
replace the symlink instead of updating the canonical target.
## Disposition
The v6 setup script renderer now owns the root fix in the generated shell
contract:
- The PVE setup script resolves `/root/.ssh/authorized_keys` with `readlink -f`
before install or uninstall edits.
- Both install and uninstall paths use the resolved path, not the symlink path.
- Both paths write through a `/tmp/.pulse-authorized-keys.*` temp file and use a
shared install helper with a copy fallback when a direct move cannot update the
target filesystem.
- The uninstall path removes all Pulse-managed SSH key lines by matching
`# pulse-`, so the current `# pulse-sensors` key is also removed.
- The forced-command key quoting fix remains covered by the existing generated
script assertion.
## Proof
- `go test ./internal/api -run 'PVESetupScript(Quotes|Preserves)|Contract_SetupScriptEmbedsFailFastGuidance' -count=1`
- `go test ./internal/api -count=1 -json > /tmp/pulse-internal-api-test.json`
## Outcome
The v6 candidate no longer knowingly regresses the v5 `#1297` setup-script
fix. Temperature-monitoring setup and complete removal now preserve the
Proxmox-managed `authorized_keys` symlink while still cleaning up Pulse-managed
SSH key entries.

View file

@ -4397,6 +4397,12 @@
"kind": "file",
"evidence_tier": "managed-runtime-exercise"
},
{
"repo": "pulse",
"path": "docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-authorized-keys-symlink-2026-05-01.md",
"kind": "file",
"evidence_tier": "managed-runtime-exercise"
},
{
"repo": "pulse",
"path": "docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-blocked-2026-05-01.md",

View file

@ -179,6 +179,10 @@ profile and assignment columns, but embedded table framing must route through
deleted, and the short recent-setup-token grace window must remain valid for
that follow-up uninstall so operators can immediately back out a reviewed
script-managed Proxmox setup without leaving a stale node record in Pulse.
That same lifecycle-owned setup path also owns Proxmox `authorized_keys`
symlink preservation for temperature-monitoring SSH keys: generated PVE
setup scripts must resolve the real authorized-keys target before filtering
Pulse-managed `# pulse-` lines during install or removal.
That same shared `internal/api/` adjacency does not transfer ownership of
AI provider setup or mobile Patrol-provider bridging:
`internal/api/ai_handlers.go` and `internal/api/chat_service_adapter.go`
@ -2095,6 +2099,11 @@ lane-local IP-pattern guess that drifts from `buildPulseMonitorTokenName`.
The generated PBS setup-script boundary must preserve that same cleanup
discovery contract instead of keeping a separate IP-pattern matcher for old
token cleanup.
The same generated PVE setup-script boundary must also preserve
Proxmox-managed `/root/.ssh/authorized_keys` symlinks when lifecycle setup or
removal touches Pulse-managed temperature-monitoring SSH keys: scripts must
resolve the real authorized-keys target before filtering `# pulse-` entries and
must use that resolved path for both install and uninstall edits.
That same generated setup-script boundary must also use exact token-name
matching when it decides whether to rotate an existing Pulse-managed token, so
reruns do not treat partial-name collisions as the canonical managed token.

View file

@ -326,6 +326,11 @@ the canonical monitored-system blocked payload.
11. Add or change infrastructure operations token generation, lookup, assignment, the pure unified-agent inventory/install model, the split infrastructure install state owner, the split direct-node/discovery infrastructure settings owners, the shared infrastructure-operations state provider/context shell, and install presentation through `frontend-modern/src/components/Settings/infrastructureOperationsModel.tsx`, `frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts`, `frontend-modern/src/components/Settings/useInfrastructureDiscoveryRuntimeState.ts`, `frontend-modern/src/components/Settings/useInfrastructureInstallState.tsx`, and `frontend-modern/src/components/Settings/useInfrastructureOperationsState.tsx`. Phase 9 retired the InfrastructureOperationsController shell and the useInfrastructureReportingState reporting path; they must not be reintroduced, and aggregator-backed reporting reads are owned by `frontend-modern/src/components/Settings/useConnectionsLedger.ts` under the frontend-primitives contract.
That same governed infrastructure-operations API boundary also owns discovery polling activation: the shared discovery runtime may only poll `/api/discover` while the settings shell has the `infrastructure-connections` route active, so route-level IA changes cannot silently keep discovery traffic alive on unrelated systems or install screens.
That same governed setup/install boundary also owns uninstall convergence: when a script-managed Proxmox node removes its local Pulse credentials, the canonical `/api/auto-unregister` API must remove the matching stored node immediately and emit the same discovery/node-deleted refresh semantics as manual deletion, so the infrastructure sources table does not keep a stale active row until the next failed poll.
That same governed setup/install boundary also owns Proxmox
`authorized_keys` symlink preservation in the rendered PVE setup script:
temperature-key install and uninstall edits must resolve the real
authorized-keys target before filtering Pulse-managed `# pulse-` lines,
and `internal/api/contract_test.go` must pin the generated shell shape.
12. Keep `internal/api/session_store.go` on a fail-closed auth-persistence boundary: persisted OIDC refresh tokens may only round-trip through encrypted-at-rest session payloads, and any missing-crypto or invalid-ciphertext path must drop the token instead of preserving plaintext-at-rest session state.
13. Keep tenant AI handler wiring on canonical provider ownership: `internal/api/ai_handlers.go` may wire tenant `ReadState` and tenant-scoped unified-resource providers into AI services, but it must not revive tenant snapshot-provider bridges once Patrol can initialize and verify from those canonical providers directly.
14. Keep Patrol status transport semantics explicit in that same AI handler layer: the Patrol status endpoint must carry machine-readable runtime availability such as blocked, running, disabled, active, or unavailable rather than asking frontend consumers to infer operator state from stale summaries or run history.
@ -2556,6 +2561,12 @@ That same generated-script payload must also remove discovered legacy tokens
from the concrete `pve` and `pam` token lists it already enumerated, rather
than iterating an undefined shell variable and silently turning operator-chosen
cleanup into a no-op.
That same generated PVE setup-script payload must also preserve Proxmox-managed
`/root/.ssh/authorized_keys` symlinks when it installs or removes Pulse
temperature-monitoring SSH keys: the rendered shell must resolve the symlink
target before filtering Pulse-managed `# pulse-` entries, use the resolved path
for both install and uninstall edits, and keep this behavior pinned in
`internal/api/contract_test.go`.
That same generated-script payload must also preserve the canonical encoded
rerun URL contract: embedded `SETUP_SCRIPT_URL` values must carry the exact
selected `host`, `pulse_url`, and `backup_perms` query state instead of

View file

@ -132,6 +132,11 @@ state.
through the shared `Table` wrapper rather than page-local scroll divs or
inline overflow styles.
4. Route transport changes for storage and recovery endpoints through `internal/api/` and the owning `api-contracts` proof routes
Shared setup-script transport may be reused by storage and recovery-linked
setup flows, but it remains API/lifecycle-owned: generated PVE scripts must
preserve Proxmox `authorized_keys` symlinks by resolving the target before
filtering Pulse-managed `# pulse-` SSH key entries, instead of letting
recovery-adjacent setup replace the symlink path with a local file.
Shared diagnostics routes may include Docker and Podman agent health notes,
but storage/recovery does not own a recovery-local runtime vocabulary for
those notes. Recovery-adjacent diagnostics consumers must preserve the
@ -2139,6 +2144,11 @@ Pulse-managed token prefix for the active Pulse URL, so adjacent setup flows
do not drift onto IP-pattern token matching that misses hostname-scoped legacy
tokens. That dependency applies to both generated PVE and PBS setup scripts,
so adjacent setup flows do not fork cleanup discovery rules by node type.
That same generated PVE setup-script dependency also assumes temperature-key
setup and removal preserve Proxmox-managed `/root/.ssh/authorized_keys`
symlinks: adjacent storage and recovery setup flows may depend on the shared
script renderer, but they must not replace the symlink path with a local file
when filtering Pulse-managed `# pulse-` SSH key entries.
That same shared discovery dependency also assumes runtime discovery state owns
only structured errors, while adjacent API and WebSocket payloads may derive
the deprecated string `errors` list only as a compatibility field from those

View file

@ -184,6 +184,67 @@ func TestPVESetupScriptQuotesTemperatureMonitoringAuthorizedKeyEntry(t *testing.
}
}
func TestPVESetupScriptPreservesAuthorizedKeysSymlink(t *testing.T) {
tempDir := t.TempDir()
cfg := &config.Config{
DataPath: tempDir,
ConfigPath: tempDir,
}
handlers := newTestConfigHandlers(t, cfg)
req := httptest.NewRequest(
http.MethodGet,
"/api/setup-script?type=pve&host=https://node.example.internal:8006&pulse_url=https://pulse.example.com:7655",
nil,
)
rr := httptest.NewRecorder()
handlers.HandleSetupScript(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("expected 200 OK, got %d (%s)", rr.Code, rr.Body.String())
}
script := rr.Body.String()
required := []string{
`resolve_authorized_keys_path() {`,
`if [ -L "$auth_keys" ]; then`,
`resolved="$(readlink -f "$auth_keys" 2>/dev/null || true)"`,
`install_authorized_keys_file() {`,
`chmod --reference="$auth_keys" "$tmp_auth_keys" 2>/dev/null || chmod 600 "$tmp_auth_keys"`,
`if mv -f "$tmp_auth_keys" "$auth_keys" 2>/dev/null; then`,
`if cp -f "$tmp_auth_keys" "$auth_keys" 2>/dev/null; then`,
`grep -vF '# pulse-' "$AUTH_KEYS" > "$TMP_AUTH_KEYS" 2>/dev/null`,
`grep -vF "# pulse-" "$AUTH_KEYS" > "$TMP_AUTH_KEYS" 2>/dev/null`,
`printf '%s\n' "$SSH_SENSORS_KEY_ENTRY" >> "$TMP_AUTH_KEYS"`,
`install_authorized_keys_file "$TMP_AUTH_KEYS" "$AUTH_KEYS"`,
}
for _, want := range required {
if !containsString(script, want) {
t.Fatalf("expected setup script to contain %q, got:\n%s", want, truncate(script, 1200))
}
}
if got := strings.Count(script, `AUTH_KEYS="$(resolve_authorized_keys_path)"`); got < 2 {
t.Fatalf("expected setup script install and uninstall paths to resolve authorized_keys symlinks, count=%d\n%s", got, truncate(script, 1200))
}
forbidden := []string{
`grep -vF '# pulse-managed-key'`,
`/root/.ssh/authorized_keys.tmp`,
`mv /root/.ssh/authorized_keys.tmp /root/.ssh/authorized_keys`,
`mv "$TMP_AUTH_KEYS" /root/.ssh/authorized_keys`,
`echo "$SSH_SENSORS_KEY_ENTRY" >> /root/.ssh/authorized_keys`,
`chmod --reference=/root/.ssh/authorized_keys`,
`chown --reference=/root/.ssh/authorized_keys`,
}
for _, stale := range forbidden {
if containsString(script, stale) {
t.Fatalf("setup script preserved stale authorized_keys handling %q:\n%s", stale, truncate(script, 1200))
}
}
}
func TestHandleSetupScriptRejectsPBSBackupPerms(t *testing.T) {
tempDir := t.TempDir()
cfg := &config.Config{

View file

@ -4477,6 +4477,30 @@ func TestContract_SetupScriptEmbedsFailFastGuidance(t *testing.T) {
if strings.Contains(script, `grep -q "$TOKEN_NAME"`) {
t.Fatalf("setup script preserved stale broad PVE token rotation detection: %s", script)
}
if !strings.Contains(script, `resolve_authorized_keys_path() {`) {
t.Fatalf("setup script missing authorized_keys symlink resolver: %s", script)
}
if !strings.Contains(script, `resolved="$(readlink -f "$auth_keys" 2>/dev/null || true)"`) {
t.Fatalf("setup script missing canonical authorized_keys target resolution: %s", script)
}
if !strings.Contains(script, `install_authorized_keys_file() {`) {
t.Fatalf("setup script missing shared authorized_keys install helper: %s", script)
}
if strings.Count(script, `AUTH_KEYS="$(resolve_authorized_keys_path)"`) < 2 {
t.Fatalf("setup script install and uninstall paths must both resolve authorized_keys symlinks: %s", script)
}
if !strings.Contains(script, `grep -vF '# pulse-' "$AUTH_KEYS" > "$TMP_AUTH_KEYS" 2>/dev/null`) {
t.Fatalf("setup script uninstall path must remove all Pulse-managed SSH keys through the resolved authorized_keys target: %s", script)
}
if !strings.Contains(script, `grep -vF "# pulse-" "$AUTH_KEYS" > "$TMP_AUTH_KEYS" 2>/dev/null`) {
t.Fatalf("setup script install path must filter existing Pulse-managed SSH keys through the resolved authorized_keys target: %s", script)
}
if strings.Contains(script, `grep -vF '# pulse-managed-key'`) {
t.Fatalf("setup script preserved stale managed-key-only cleanup: %s", script)
}
if strings.Contains(script, `/root/.ssh/authorized_keys.tmp`) || strings.Contains(script, `echo "$SSH_SENSORS_KEY_ENTRY" >> /root/.ssh/authorized_keys`) {
t.Fatalf("setup script preserved symlink-breaking authorized_keys rewrite: %s", script)
}
if strings.Contains(script, `echo "Please run this script as root"`) {
t.Fatalf("setup script preserved stale root-only guidance: %s", script)
}

View file

@ -65,6 +65,44 @@ if [ "$EUID" -ne 0 ]; then
exit 1
fi
resolve_authorized_keys_path() {
local auth_keys="/root/.ssh/authorized_keys"
local resolved=""
if [ -L "$auth_keys" ]; then
resolved="$(readlink -f "$auth_keys" 2>/dev/null || true)"
if [ -n "$resolved" ]; then
auth_keys="$resolved"
fi
fi
printf '%%s\n' "$auth_keys"
}
install_authorized_keys_file() {
local tmp_auth_keys="$1"
local auth_keys="$2"
if [ ! -f "$tmp_auth_keys" ]; then
return 1
fi
chmod --reference="$auth_keys" "$tmp_auth_keys" 2>/dev/null || chmod 600 "$tmp_auth_keys"
chown --reference="$auth_keys" "$tmp_auth_keys" 2>/dev/null || true
if mv -f "$tmp_auth_keys" "$auth_keys" 2>/dev/null; then
return 0
fi
if cp -f "$tmp_auth_keys" "$auth_keys" 2>/dev/null; then
rm -f "$tmp_auth_keys"
return 0
fi
rm -f "$tmp_auth_keys"
return 1
}
# Detect environment (Proxmox host vs LXC guest)
detect_environment() {
if command -v pveum >/dev/null 2>&1 && command -v pveversion >/dev/null 2>&1; then
@ -241,23 +279,25 @@ if [[ $MAIN_ACTION =~ ^[2Rr]$ ]]; then
# Always run manual removal for local services and files
if true; then
# Remove SSH keys from authorized_keys (only Pulse-managed entries)
if [ -f /root/.ssh/authorized_keys ]; then
AUTH_KEYS="$(resolve_authorized_keys_path)"
if [ -f "$AUTH_KEYS" ]; then
echo " • Removing SSH keys from authorized_keys..."
TMP_AUTH_KEYS=$(mktemp)
if [ -f "$TMP_AUTH_KEYS" ]; then
grep -vF '# pulse-managed-key' /root/.ssh/authorized_keys > "$TMP_AUTH_KEYS" 2>/dev/null
TMP_AUTH_KEYS="$(mktemp /tmp/.pulse-authorized-keys.XXXXXX 2>/dev/null)" || TMP_AUTH_KEYS=""
if [ -n "$TMP_AUTH_KEYS" ] && [ -f "$TMP_AUTH_KEYS" ]; then
grep -vF '# pulse-' "$AUTH_KEYS" > "$TMP_AUTH_KEYS" 2>/dev/null
GREP_EXIT=$?
if [ $GREP_EXIT -eq 0 ] || [ $GREP_EXIT -eq 1 ]; then
chmod --reference=/root/.ssh/authorized_keys "$TMP_AUTH_KEYS" 2>/dev/null || chmod 600 "$TMP_AUTH_KEYS"
chown --reference=/root/.ssh/authorized_keys "$TMP_AUTH_KEYS" 2>/dev/null || true
if mv "$TMP_AUTH_KEYS" /root/.ssh/authorized_keys; then
if install_authorized_keys_file "$TMP_AUTH_KEYS" "$AUTH_KEYS"; then
:
else
rm -f "$TMP_AUTH_KEYS"
echo " ⚠️ Failed to update $AUTH_KEYS"
fi
else
rm -f "$TMP_AUTH_KEYS"
fi
else
echo " ⚠️ Failed to create temporary authorized_keys file"
fi
fi
@ -596,18 +636,40 @@ if [ -n "$SSH_SENSORS_PUBLIC_KEY" ]; then
echo "Configuring temperature monitoring..."
# Add key to root's authorized_keys
AUTH_KEYS="$(resolve_authorized_keys_path)"
mkdir -p "$(dirname "$AUTH_KEYS")"
mkdir -p /root/.ssh
chmod 700 /root/.ssh
chmod 700 /root/.ssh 2>/dev/null || true
# Remove any old pulse keys
if [ -f /root/.ssh/authorized_keys ]; then
grep -vF "# pulse-" /root/.ssh/authorized_keys > /root/.ssh/authorized_keys.tmp 2>/dev/null || touch /root/.ssh/authorized_keys.tmp
mv /root/.ssh/authorized_keys.tmp /root/.ssh/authorized_keys
fi
SENSORS_KEY_OK=false
TMP_AUTH_KEYS="$(mktemp /tmp/.pulse-authorized-keys.XXXXXX 2>/dev/null)" || TMP_AUTH_KEYS=""
if [ -n "$TMP_AUTH_KEYS" ] && [ -f "$TMP_AUTH_KEYS" ]; then
if [ -f "$AUTH_KEYS" ]; then
grep -vF "# pulse-" "$AUTH_KEYS" > "$TMP_AUTH_KEYS" 2>/dev/null
GREP_EXIT=$?
if [ $GREP_EXIT -ne 0 ] && [ $GREP_EXIT -ne 1 ]; then
echo " ⚠️ Failed to read $AUTH_KEYS"
rm -f "$TMP_AUTH_KEYS"
TMP_AUTH_KEYS=""
fi
fi
echo "$SSH_SENSORS_KEY_ENTRY" >> /root/.ssh/authorized_keys
chmod 600 /root/.ssh/authorized_keys
echo " ✓ Sensors key configured (restricted to sensors -j)"
if [ -n "$TMP_AUTH_KEYS" ]; then
printf '%%s\n' "$SSH_SENSORS_KEY_ENTRY" >> "$TMP_AUTH_KEYS"
if install_authorized_keys_file "$TMP_AUTH_KEYS" "$AUTH_KEYS"; then
SENSORS_KEY_OK=true
else
echo " ⚠️ Failed to update $AUTH_KEYS"
rm -f "$TMP_AUTH_KEYS" 2>/dev/null || true
fi
fi
else
echo " ⚠️ Failed to create temp file - cannot update authorized_keys"
fi
if [ "$SENSORS_KEY_OK" = true ]; then
echo " ✓ Sensors key configured (restricted to sensors -j)"
fi
# Check if this is a Raspberry Pi
IS_RPI=false