fix: match identities against the record, not the name shape (audit theme 5)

[passcod]

Closes cross-cutting theme 5 from the logic bug audit: name/prefix matching without reserved namespaces. See the pattern analysis.``

The class

Seedling grants itself identifiers inside namespaces it does not own exclusively, then recognises "its" objects later by string shape — or, in one case, by eight bits of an ID. One discipline violated at two different points: identity must be granted once, recorded, and every later match made against the record. Sub-class (a) violates it at match time; sub-class (b) at grant time.

(a) Exact-identity matching for uninstall — H2

run_uninstall_phase recognised an app's units with list_units("seedling-{app}-"), a plain starts_with. Unit names are seedling-{display_name}.service where display_name is {app}-{name}[-{suffix}], and both app names and resource names may contain hyphens — so the encoding is not prefix-free. Uninstalling app matched every unit of a sibling called app-db, and the retry branch then reset_failed_unit'd and stop_unit'd that healthy sibling's units every five-second tick, while the uninstall never completed.

The irony is that the exact identities were already recorded: resource_instances rows hold every display_name the app ever actuated, and they are deleted only at uninstall completion — so they are available for the entire window in which the match runs. The prefix match re-derived, lossily, what the registry already knew losslessly.

The prefix scan now only enumerates candidates; the decision is an exact match against the recorded set. app-db's units can never appear, with no constraint on operator naming and no renaming of deployed units — the format stays exactly as it is, so there is no restart churn and no window where the observer loses sight of running containers.

(b) Reserved namespaces, and ownership at the destructive consumers

crates/core/src/reserved.rs is the one home for names the daemon claims: the backup-snap- site-volume prefix and the tailscale site-ingress name. They lived apart before — backup_execution.rs and tailscale.rs knew nothing about each other or about the creation handlers — which is how the gap opened. Creation rejects them (create_site_volume, restore_held, snapshot, promote, create_site_ingress), creation only, so an operator with a legacy object can still rename or remove it.

Reservation alone is not enough: it cannot repair a collision that already exists, and does not protect against a future path that forgets to ask. So the destructive consumers check recorded ownership too:

• the startup snapshot sweep skips names present in site_volumes — a DB row is the record that an operator owns the name;
• mark_existing_stale and upsert_discovered_row act only on rows whose source.is_discovered(). A manual ingress named tailscale was previously disabled on the provider's first bad poll and never re-enabled, and the upsert then collided on the primary key every poll — livelocking, so the operator's row stayed stale forever. It now declines and says why.

Findings closed

Finding	Severity
H2 — uninstall stops sibling apps whose names extend the uninstalling app's	high
Operator site volumes named backup-snap-* destroyed by startup cleanup	medium
mark_existing_stale/upsert have no ownership check, so a manual tailscale ingress is permanently disabled	medium
H12 — all static Jobs share one pod /64	high (deterministic half)

H12 is only half fixed, deliberately

Static Jobs carry a nil InstanceId, so deriving the pod /64's low byte from uuid[0] gave every static Job on the node, across every app, the identical subnet — and netavark rejects the second network on a duplicate subnet. That deterministic collision is gone: the low byte is now a hash of the full identity (app, kind, name, whole UUID), so two distinct Jobs differ even when both UUIDs are nil.

The probabilistic half is untouched: it is still eight bits, so scaled replicas of one deployment birthday-collide at the same rate as before. Widening it means taking byte 6 as well, and byte 6 is the kind discriminant that keeps pod /64s disjoint from the service /128 space and the fffe infrastructure addresses — not something to overload without deciding what those namespaces mean, and not something I could validate here. The real fix is allocation rather than derivation: an instance_id → subnet id table with a unique index, allocated at first actuation and freed at GC, which needs a database handle in what is currently a pure translate layer. Left as the next step rather than half-done, and called out in the function's doc comment. Sixteen bits is too small for a hash to masquerade as allocation at fleet scale — even a perfect one gives roughly 7% collision odds at 100 concurrent instances.

Changing the derivation re-homes each instance at its next pod recreation, when its per-instance network is torn down and remade. No flag day.

Enforcement

• Spec: r[namespace.reserved] (which names are reserved, that the restriction is creation-only, and that destructive consumers must identify their objects by what was recorded regardless of name), r[app.uninstall.scope] (uninstall affects only that app's resources; names are not required to be prefix-free; a prefix scan may only enumerate candidates that are then matched exactly), r[infra.pod.subnet].
• Tests: reserved names rejected at creation while a name merely containing the prefix is allowed; display_names_for_app scoped to its own app, with the test asserting the sibling really does match the old prefix so the regression can't quietly stop reproducing; static Jobs with nil ids getting distinct subnets; the pod prefix keeping its kind discriminant.

Not in scope

Collisions with objects Seedling never recorded — a unit or podman network some other tool names seedling-… is still swept by orphan cleanup, because "not in the registry" is indistinguishable from "orphaned". That residual is inherent to sharing host namespaces and belongs in operator docs. Also out: the other Tailscale finding (never marking the discovered ingress stale on Unreachable, which is missing behaviour rather than misattributed identity), and name reuse over time — external mappings inherited by a later app of the same name are a lifecycle-cleanup bug. Reservation scopes namespaces in space, not in time.

Overlap with other themes

Independent — sits directly on main. It touches runtime/tailscale.rs, which theme 4 (#140) also edits for the tailscale_unreachable fault, but in different functions; expect a trivial merge either way.

---

Generated by Claude Code

[github-code-quality]

Code Coverage Overview

Languages: TypeScript, Rust

TypeScript / code-coverage/vitest

The overall coverage in commit bf66abf in the claude/pr-115-theme-... branch remains at 66%, unchanged from commit b383126 in the main branch.

Rust / code-coverage/rust

The overall coverage in commit bf66abf in the claude/pr-115-theme-... branch remains at 59%, unchanged from commit b383126 in the main branch.

Show a code coverage summary of the most impacted files.

File	main b383126	claude/pr-115-theme-... bf66abf	+/-
crates/core/src/oi/server.rs	60%	59%	-1%
crates/core/src...dler/volumes.rs	74%	74%	0%
crates/core/src...me/tailscale.rs	40%	40%	0%
crates/core/src...time/history.rs	82%	82%	0%
crates/core/src...er/ingresses.rs	78%	78%	0%
crates/core/src...em/reconcile.rs	25%	26%	+1%
crates/core/src...nslate/proxy.rs	46%	53%	+7%
crates/core/src/reserved.rs	0%	100%	+100%

---

_{Updated August 02, 2026 03:25 UTC}

[Copilot]

Pull request overview

This PR closes audit theme 5 by shifting “is this ours?” checks from name-shape heuristics to recorded identity/ownership, and by centralising reserved-name rules so creation-time reservation and destructive consumers both respect operator-owned objects.

Changes:

• Adds spec requirements for reserved namespaces, uninstall scoping, and pod subnet derivation.
• Introduces a reserved module and enforces reserved-name rejection at creation for site volumes and site ingresses, plus ownership checks in destructive consumers (snapshot sweep, Tailscale discovery).
• Fixes uninstall unit targeting to match exactly against resource_instances-recorded identities and improves pod /64 derivation for static Jobs with nil instance IDs.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
docs/spec/runtime.md	Adds spec items for reserved namespaces, uninstall scoping, and pod subnet behaviour.
crates/daemon/src/main.rs	Makes snapshot startup cleanup skip operator-registered site volumes even if they match the snapshot prefix.
crates/core/src/system/translate/proxy.rs	Changes pod /64 derivation to hash full identity to avoid deterministic collisions for nil-id Jobs.
crates/core/src/system/reconcile.rs	Makes uninstall match systemd units by recorded identity rather than lossy prefix matching.
crates/core/src/runtime/tailscale.rs	Prevents discovery from disabling/replacing operator-owned tailscale ingress rows.
crates/core/src/runtime/history.rs	Adds display_names_for_app helper to query recorded instance display names for an app.
crates/core/src/runtime/history/tests.rs	Adds regression test asserting display-name scoping prevents sibling-app uninstall collisions.
crates/core/src/reserved.rs	Centralises reserved site-volume prefixes and site-ingress names plus creation-time checks and unit tests.
crates/core/src/oi/handler/volumes.rs	Enforces reserved site-volume name checks at creation/restore/snapshot/promote handlers.
crates/core/src/oi/handler/volumes/tests.rs	Adds OI test that reserved snapshot prefix is rejected at creation while contained prefix is allowed.
crates/core/src/oi/handler/ingresses.rs	Enforces reserved site-ingress name check at creation.
crates/core/src/oi/handler/ingresses/tests.rs	Adds OI test that tailscale site ingress name is rejected at creation.
crates/core/src/lib.rs	Exposes the new reserved module.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

crates/core/src/runtime/tailscale.rs:413

• upsert_discovered_row queries site_ingresses::get(db, &name)? twice back-to-back. This adds an extra DB round-trip and makes the control flow harder to follow; you can fetch once and then branch on the result.

    if let Some(existing) = site_ingresses::get(db, &name)?
        && !existing.source.is_discovered()
    {
        warn!(
            "tailscale: a manually-created site ingress holds the name \

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/core/src/system/reconcile.rs:1487

• list_units("seedling-") enumerates units for all apps and then filters; you can keep the exact-match safety while reducing work by scanning with the uninstalling app’s prefix (seedling-{app}-) and then matching against the recorded expected set. This aligns with the spec’s “prefix scan only enumerates candidates” rule without widening the candidate set unnecessarily.

            match self
                .driver
                .process
                .list_units("seedling-")
                .await

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (2)

crates/daemon/src/main.rs:669

• The info log inside the startup snapshot sweep says "a registered site volume…" for every kept name, but owned also includes names kept due to parse/DB errors ("could not tell"). That makes this log message inaccurate/misleading for non-registered volumes that are merely being kept conservatively.

                        let keep = owned.contains(name.as_str());
                        if keep {
                            tracing::info!(
                                volume = %name,
                                "a registered site volume uses the backup snapshot prefix; \
                                 leaving it alone"
                            );

crates/core/src/runtime/history.rs:94

• display_names_for_app can return duplicate display_name values if an app reuses a name over time (new instance IDs with the same display_name). Callers (like uninstall) immediately de-duplicate into a HashSet, so returning duplicates adds unnecessary DB/result-set work.

    let mut stmt = db
        .conn
        .prepare("SELECT display_name FROM resource_instances WHERE app = ?1")?;
    let rows = stmt.query_map(params![app], |row| row.get::<_, String>(0))?;

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

crates/daemon/src/main.rs:668

• This log line says the volume is “registered”, but the owned set also includes names kept due to parse failures or DB errors. That makes the message misleading in exactly the cases where operators most need accuracy.

                        Some(Keep::Registered) => {
                            tracing::info!(
                                volume = %name,
                                "a registered site volume uses the backup snapshot prefix; \
                                 leaving it alone"

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/daemon/src/main.rs:654

• This branch treats an invalid SiteVolumeName as "not a snapshot of ours" and skips deletion, but backup-execution snapshot names are not validated against the bsl.name/SiteVolumeName rules (and can exceed the 63-char limit if the strategy name is long). That means legitimate orphan backup snapshots can become undeletable and accumulate across restarts.

                                // Also a "could not tell": a name Seedling
                                // could not have created is not a snapshot of
                                // ours to delete.
                                Err(e) => Keep::UnparseableName(e.to_string()),