[stephrobert]

Summary

Attach asked one of the two questions this layer has to ask. It checked that
its machine name and its network name were well formed (safeName) and
stopped there. Detach, three hundred lines away, asks both.

Both names come from Resource.Runtime. PUT /_feint/state and feint snapshot load restore that map verbatim, and internal/cli/snapshot.go
documents the format as meant to outlive its instance and be loaded into
another one. So a crafted snapshot naming an instance of the operator reached:

incus config device add <that instance> ethN nic network=...

production-database passes safeName. That is the whole point of the
repository's "bien formé n'est pas autorisé" rule: a syntax check was standing
where an ownership check belonged, and this is the third time that exact
substitution has been found here.

The change

internal/core/machine/incus.go, in Attach:

• the network must satisfy ownedNetwork, not merely safeName;
• the machine must satisfy d.mustOwnInstance(ctx, name), which reads the
  user.feint.provider label the emulator itself wrote at create;
• the instance check sits inside the per-machine lock and before
  waitForAgent, so Attach sends nothing to a foreign instance.

One word of precision, because the first draft of this overstated it: that is
true of Attach, not of the whole join. Reconciler.Join runs its
read-back whether the attach was refused or not, and Observe still execs into
the instance without asking ownership. That is #920, already open, and it is
not closed by this lot.

One deliberate difference from Detach: a missing instance is an error here,
not nil. A detach of a machine that is gone has nothing left to undo. An
attach that answered nil would let the caller go on and publish an address for
a machine that does not exist.

Type of change

• A route added or changed
• Bug fix
• Refactor
• Documentation
• Chore / tooling
• Security / supply chain

The tests, and the accepting half

• TestAttachRefusesAnInstanceItDoesNotOwn
• TestAttachRefusesANetworkOutsideThePrefix
• TestAttachStillReachesAnInstanceTheEmulatorCreated

The third one is not decoration. A guard that refuses everything passes every
attack test and breaks the product, so the suite asserts the accepting half
explicitly.

All three go through the injectable runner, which records argv. That is
what lets a test assert the property that actually matters: no emitted
command carries the foreign name. No machine was started to prove any of
this.

Runs

run	verdict
rebase onto main (ce59308, after #929)	clean, e97ca46, diff unchanged
mise run prepush	green, 119 s, on the rebase
the 16 falsify specs mise run testplan names	all 16 exit 0
FEINT_VM=incus-ovn mise run conformance:leg -- runtime	green, 1435 s
station afterwards	0 machine, 0 fnt- network, no surviving DHCP

Mode, stated because a blank cell has been read as "not asked" before:
incus-ovn, on the maintainer's station, checked free first (free -g,
incus list) with no labs running.

The leg's own closing checks, from its log: no machine or network of this run is left on this runtime, nothing on this runtime is beyond an ordinary sweep, no DHCP service of this emulator's outlives its network on this host.

On the leg's cost

CLAUDE.md still announces 590 s for this leg. Two independent measurements a
day apart give 1433 s (2026-11-13) and 1435 s (2026-11-14). The gap is
not noise, and part of it is identified: six machine starts each wait 90 s for
a resolver the guest never names (incus_resolver.go), which is nine minutes
of expiring waits the leg passes without. Recorded here, not fixed here.

Falsification

tools/falsify/specs/attach-asks-both-questions.json, new, shows each of the
three tests turns red without the guard it holds. Two older specs,
attach-per-machine.json and run-leaves-nothing.json, had their find
fragments retargeted around the new lines; both replay green in the 16 above.

Runs not played

The full mise run conformance pass. The runtime leg carries the four suites
a driver change touches, which is the row mise run testplan names, and it ran
under the mode that can actually prove isolation. What the full pass would add
here is the client suites, which this diff does not reach.

Checklist

Always

• mise run check passes (inside prepush, on the rebase)
• No new external Go dependency
• internal/core still knows no provider. mustOwnACL reads a description
  rather than a name precisely so no provider spelling enters the core, and
  this change keeps that
• Nothing in the diff could be written identically for another provider:
  the guard is at the shared choke point, so all three packs get it at once
• mise run testplan was run, and what it printed was run. The one run not
  played is named above

When a model wrote a substantive part of this

• An Assisted-by: trailer names the tool and the model version
• The leg was run, not assumed, and its measured time is in the table
• No field name in the diff: no response shape changes

When a route is added or changed

N/A.

When behaviour a client can observe changes

N/A. A refusal that was already supposed to happen now happens. No documented
limit moves.

When .github/workflows/ is touched

N/A.

When a machine runtime is involved

• Tested with FEINT_VM=incus-ovn
• Nothing the emulator did not create was touched: that is the subject of
  the change, and the runner tests assert it at argv level
• The run leaves nothing behind, checked: incus list and incus network list after the leg, plus the leg's own three closing verdicts

Review

Security review: reported, no blocker on this commit. Attach is closed.
The order was checked call by call: nothing is emitted before the ownership
probe, the check sits inside the lock that protects the action it guards, and a
failed probe fails closed (daemon down, timeout and permission denied all
refuse, and isNotFound deliberately carries no bare "not found" that would
degrade them to permissive).

It also confirmed that the guard's own input is not attacker-writable: instance
labels are written only at incus init, from a derived name, and the
existing-instance branch of Start writes no config, so there is no adoption
path.

It found three sibling paths this lot does not touch. Each has a home
rather than a sentence here:

• The cold twin of #909: a boot puts our machine on the operator's own network #932, the cold twin of this one, and the one that matters: incus.go:508
  and incus.go:704 accept any well-formed network name from a restored
  Runtime. Measured through the runner: init ... --network incusbr0 and
  config device add ... network=incusbr0. That is the boot path, so it runs
  more often than the hot attach this lot fixes.
• RouteAddress's managed branch asks the network and never the instance it writes to #933, RouteAddress's managed branch asks the network and never the
  instance it writes to. Its own comment says so, which is the defect.
• Observe and Door run inside an instance whose ownership nobody asked about #920, already open, Observe and Door.

#932 is the reason to read this pull request narrowly: it closes Attach, not
the attach family.

Falsifier: reported, no blocker. Nine mutations measured, each built and
vet-clean, baseline green and "after restoration: green" in every pass.

It found one real weakness in this lot, and ad6e6fb fixes it:
TestAttachRefusesANetworkOutsideThePrefix stayed green against
ownedNetwork(name) in place of ownedNetwork(att.Network), because every row
drove the machine feint-scw-mine, which ownedNetwork also refuses. The
refusal happened, for the wrong reason, and only the accepting half caught the
defect. The table now carries the machine name and its last row gives it one
ownedNetwork accepts. The mutation was added to the spec, which is now six,
all biting; the reviewer re-measured on ad6e6fb and confirms it bites there
and did not on e97ca46.

Two findings outside this lot, each with its own issue:

• Attach's ownership answer is used under a lock nothing says it must be taken under #934: the ownership answer is used under a lock nothing says it must be
  taken under. Measured over the whole package, a completed run with no
  timeout: no test catches the check moving out of the lock, where all five
  other plausible defects in this guard are caught. Not a vulnerability today
  (nothing relabels an existing instance) and not a broken promise (neither the
  commit nor the comment claims the lock placement). That is the issue: the
  property holds by accident of what no other path does.
• A guard that refuses early makes the start-race tests hang instead of fail #935: five tests in incus_start_race_test.go wait on a bare channel
  with no deadline, so a guard that refuses before the device add makes them
  hang rather than fail. It truncated three of the reviewer's own
  measurements, which is the harness failing before the subject.

One measurement-integrity note the review raised, worth recording: the shared
test fake answers "feint\n" to any ownership probe by default
(incus_firewall_test.go:53). This lot's tests override it, so they are sound.
The next ownership guard somebody writes will look tested when it is not.

Related issues

Closes #909.

🤖 Generated with Claude Code