Skip to content

core: harden instance attachment and typed-nil boundaries - #1

Draft
Joshua Tenner (jtenner) wants to merge 6 commits into
mainfrom
agent/harden-instance-lifecycle
Draft

core: harden instance attachment and typed-nil boundaries#1
Joshua Tenner (jtenner) wants to merge 6 commits into
mainfrom
agent/harden-instance-lifecycle

Conversation

@jtenner

@jtenner jtenner commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

This hardens instance lifecycle ownership and several fail-closed boundaries discovered during a repository-wide correctness, coverage, and concurrency audit.

  • Add a per-instance in-flight attachment barrier so Detach cannot miss an Attach that is still constructing its namespace.
  • Reject a duplicate attach before invoking the backend factory.
  • Retire partial state, quota reservations, and the attachment barrier if namespace construction returns an error or panics; panics still propagate.
  • Clear State.NamespaceHandle whenever the resource table has retired that handle, even when the backend's Close reports an error.
  • Reject typed-nil HostModule values before calling Memory or resolving instance identity.
  • Reuse the shared IPv4 identity validator so dynamic identity updates reject loopback and limited-broadcast addresses without mutating namespace state.
  • Add focused regression coverage for each failure mode plus exact namespace lookup/retirement behavior.

Root causes

Manager.Attach previously published state only after backend construction, while Detach treated an absent published state as already detached. A concurrent detach could therefore return successfully and then allow the attach to publish an orphaned state. The same construction window also allowed a duplicate backend factory invocation and left no panic-safe release path.

resource.Table.CloseHandle retires a handle before invoking the resource's Close; conditioning State.NamespaceHandle cleanup on a nil close error left a stale advertised handle after retirement.

Plain interface nil checks do not detect typed-nil pointers. Calling methods through those interfaces could panic instead of failing closed.

The dynamic IPv4 path duplicated only part of the static identity validation and therefore admitted loopback and limited broadcast addresses.

Validation

On the full repository audit checkout at base commit 76392ea102b847a633a3cdd3e77ec54ef524dda2:

  • normal package matrix: 98/98 packages passed
  • shuffled package matrix: 98/98 packages passed
  • race-instrumented matrix: 95/95 applicable packages passed
  • go vet ./... passed
  • all 37 fuzz targets completed focused smoke runs
  • dependency-matrix and release-provenance suites passed separately
  • internal/instance/core passed 50 repeated race-instrumented runs with the lifecycle regressions

The execution workspace was recycled after those runs. I reconstructed this minimal six-file branch from the same base commit, verified the three baseline blobs byte-for-byte against GitHub, verified every submitted remote blob against the local content hash, and reran the exact reconstructed changes in an isolated semantic harness, including 20 race-instrumented repetitions for each affected package.

Scope

The branch intentionally excludes unrelated static-analysis cleanup found during the audit so this PR remains focused on correctness fixes and their regression tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant