Skip to content

test(fleet): add a write-containment guard and a two-node facts smoke test - #272

Merged
toasterbook88 merged 2 commits into
mainfrom
feat/fleet-test-containment-guard
Aug 4, 2026
Merged

test(fleet): add a write-containment guard and a two-node facts smoke test#272
toasterbook88 merged 2 commits into
mainfrom
feat/fleet-test-containment-guard

Conversation

@toasterbook88

Copy link
Copy Markdown
Owner

What

A safety boundary for fleet tests (internal/fleettest), plus the first test that uses it: a two-node read-only facts smoke, gated behind //go:build fleet with a make test-fleet target.

Why the guard exists

Fleet tests run real AXIS binaries against real nodes, so whatever AXIS_HOME resolves to is writable by the test. Exporting a suite-wide AXIS_HOME at a live store is enough for config-writing tests to rewrite it — cmd/axis/llm_test.go simulates axis llm configure and writes provider entries. That happened during development of this branch: a live ~/.axis had fixture provider config written into nodes.yaml and a secrets/ directory created.

The guard makes that unrepresentable. A fleet test takes its environment from a Guard, and a Guard cannot be constructed around a path outside its own run root.

Enforcement is programmatic rather than conventional on purpose. The Makefile already documents that AXIS_HOME outranks HOME; a comment does not stop an export on the command line, a constructor that refuses does.

Not a production helper. Nothing under cmd/ imports it — asserted with go list -deps ./cmd/axis — so it never enters the shipped binary and cannot reject a legitimate operator store during normal use. A guard wired into production resolution would either do that or be skipped by any caller that forgot it.

The guard

  • creates its own unique run root; resolves run root and AXIS_HOME physically before comparing
  • requires strict descendancy — the root itself is rejected
  • isolates HOME as well, since persist falls back to it when AXIS_HOME is empty
  • rejects empty, whitespace, relative, ..-bearing, symlink-escaping, unresolvable, dangling-link and prefix-lookalike paths
  • RemoteCommand re-verifies containment on the far side with POSIX builtins and exits 78 if it cannot confirm — the controller cannot see the remote filesystem
  • RemoteRoot derives the remote run root from the local run root's unique suffix, so overlapping runs cannot collide on a shared remote path
  • leaves run directories in place; retention is a separate concern

The smoke test

Collects facts from the controller and one remote node concurrently and asserts invariants — arch and hostname non-empty, CollectedAt present and recent, no node reporting complete alongside an error, transport failure surfacing as StatusUnreachable rather than healthy. Writes a structured JSON report under the run root.

Targets come from AXIS_FLEET_TARGET ([user@]host[:port]), with optional AXIS_FLEET_TARGET_NAME and AXIS_FLEET_CONTROLLER. It skips when unset — absent a fleet there is nothing to assert, and failing would report the code as broken when only the environment is missing.

Verification

  • make test 42/42; the fleet test does not run there (build tag confirmed)
  • skip path verified with no target configured; PASS verified against a configured target
  • gofmt and vet clean on both tagged and untagged builds
  • Adversarial table of 14 cases; two defects found while writing it — a dangling symlink was accepted, and the .. cases were built with filepath.Join, which normalises .. away before the check sees it. Both fixed
  • Mutation-checked against the real code: removing the symlink check, the .. guard, the per-node assertions, or breaking RemoteRoot each fails the corresponding tests

Caveat

Containment requires mkdir -p of a run root on the remote, so this tier is not strictly read-only against a cluster. Writes are confined to /tmp/<run-id>-<node> and never removed, consistent with the no-delete policy. Retention across repeated runs is open.

Note on merge order

Touches Makefile alongside #271 (test-fleet vs install-system/test-install). Whichever merges second will need a trivial conflict resolution.

🤖 Generated with Claude Code

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

AXIS Contributor and others added 2 commits August 3, 2026 22:44
Fleet tests run real AXIS binaries against real nodes, so whatever AXIS_HOME
resolves to is writable by the test. Pointing a suite-wide AXIS_HOME at a live
~/.axis is enough to have config-writing tests rewrite an operator's store:
llm_test.go alone simulates `axis llm configure` and writes provider entries.
This package makes that unrepresentable — a fleet test takes its environment
from a Guard, and a Guard cannot be constructed around a path outside its own
run root.

Enforcement is programmatic rather than conventional on purpose. The Makefile
already documents that AXIS_HOME outranks HOME; a comment does not stop an
export on the command line, a constructor that refuses does.

Scope: harness boundary only. Nothing under cmd/ imports it (asserted via
go list -deps), so it never enters the shipped binary and cannot reject a
legitimate operator store during normal use. A guard wired into production
resolution would either do that or be skipped by any caller that forgot it.

The guard:
  - creates its own unique run root
  - resolves run root and AXIS_HOME physically before comparing
  - requires AXIS_HOME to be a strict descendant, never the root itself
  - isolates HOME as well, since persist falls back to it when AXIS_HOME is empty
  - rejects empty, relative, '..'-bearing, symlink-escaping, unresolvable and
    prefix-lookalike paths
  - emits a remote wrapper that re-verifies containment on the far side with
    POSIX builtins and exits 78 if it cannot confirm, because the controller
    cannot see the remote filesystem
  - leaves run directories in place; retention is a later, separate concern

Two defects were found by the adversarial table while writing it:
  - a dangling symlink was accepted, because resolution walked past an
    unresolvable leaf and re-attached its name as if it were a not-yet-created
    directory
  - the '..' cases were built with filepath.Join, which normalises ".." away
    before Validate sees it, so they asserted nothing

Both fixed. Removing either the symlink check or the '..' check fails the
corresponding case, so the table distinguishes the branches rather than passing
incidentally.

No cluster access, no execution, no deployment, no cleanup behaviour.
First fleet-tier test: collects facts from the controller and one remote node
concurrently and asserts invariants. Gated behind //go:build fleet with a
separate `make test-fleet` target, so `make test` and PR CI stay hermetic and
cluster-free.

Targets come from the environment — AXIS_FLEET_TARGET ([user@]host[:port]),
with optional AXIS_FLEET_TARGET_NAME and AXIS_FLEET_CONTROLLER for reporting.
The test skips when AXIS_FLEET_TARGET is unset. A fleet test that hardcodes one
operator's hosts cannot run for anyone else, and skipping is the honest default
for a cluster-dependent test: absent a fleet there is nothing to assert, and
failing would report the code as broken when only the environment is missing.

Every environment value comes from a Guard. The remote side receives
RemoteEnv() and runs through RemoteCommand(), which re-verifies containment on
the far end before executing — the controller cannot see the remote filesystem,
so only the remote shell can confirm its AXIS_HOME is inside its run root.

Guard.RemoteRoot() derives the remote run root from the local run root's unique
suffix. A fixed per-node path meant two overlapping runs — CI plus a local run,
or the concurrent cases planned next — shared one remote directory and
interleaved writes into the same axis-home. Run isolation is the guard's whole
purpose; it has to hold on both sides.

Assertions report per-node via t.Errorf at the point of failure. An earlier
version funnelled every check through one accumulator and a single terminal
t.Error, so deleting one line silently disabled all six assertions while the
suite stayed green.

Review notes worth keeping:

  - The uniqueness assertion must call Guard.RemoteRoot, not restate the
    formula. A first version reimplemented the derivation inside the test; it
    passed while the shipped path went uncovered, and mutating it only proved
    the copy was live. The derivation now has one implementation in guard.go,
    reachable from the untagged suite. Breaking it fails `make test`.
  - Transport failure is reported as StatusUnreachable, which already exists and
    is handled in dashboard.go and snapshot.go. An earlier version added
    StatusUnknown to internal/models purely for the test's vocabulary; that is a
    production enum change with unhandled switch cases and was reverted.

Deliberately not included: remote execution, reservations, expected-service
checking (needs a policy source that does not exist), clock-skew testing,
additional platforms, and any cleanup or retention behaviour.

Note: containment requires `mkdir -p` of a run root on the remote, so this tier
is not strictly read-only against a cluster. Writes are confined to
/tmp/<run-id>-<node> and are never removed, consistent with the no-delete
policy. Retention across repeated runs is an open question.

Verified: make test 42/42, skip path with no target configured, PASS against a
configured target, gofmt and vet clean on both tagged and untagged builds, and
both the uniqueness and per-node assertions mutation-checked against the real
code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@toasterbook88
toasterbook88 force-pushed the feat/fleet-test-containment-guard branch from ef0d665 to 12e674f Compare August 4, 2026 02:46
@toasterbook88
toasterbook88 merged commit 4c6dbb7 into main Aug 4, 2026
7 checks passed
@toasterbook88
toasterbook88 deleted the feat/fleet-test-containment-guard branch August 4, 2026 02:52
toasterbook88 added a commit that referenced this pull request Aug 4, 2026
Release prep for **v0.14.9**, following the publish path documented at
the top of `.github/workflows/release.yml`: bump the version constant,
then tag. GoReleaser owns the GitHub Release.

## Changes

- `internal/buildinfo/version.go` → `0.14.9`
- `docs/current-state.md` regenerated via
`hack/refresh-current-state.sh`
- Three summary goldens carry the version string and move with the
constant. `UPDATE_GOLDEN` is only wired into the contract tests, not
these, so they were updated directly.

## Contents since v0.14.8

| PR | |
|---|---|
| #270 | two `cmd/axis` tests self-isolate `HOME`/`AXIS_HOME` instead of
relying on the caller, so a bare `go test ./...` no longer reads the
operator's store |
| #271 | `install.sh` installs system-wide by default, stages and
validates before replacing, classifies the existing entry, and gains a
committed regression suite (`make test-install`) |
| #272 | fleet-test write-containment guard plus a build-tagged two-node
facts smoke (`make test-fleet`) |

## Call out in the release notes

#271 changes `install.sh`'s default target from `$HOME/.local/bin` to
`/usr/local/bin` and removes the superseded user-local copy. That is
user-visible behaviour for anyone who reinstalls. The increment is a
patch to match the established cadence, but the behaviour change
deserves prominence.

## Verification

```
make test           42 packages ok
make test-install   36 assertions, 0 failed
verify-repo-truth   exit 0
gofmt / vet         clean
```

## Two tooling defects found while preparing this

Neither is fixed here — flagging rather than widening a release PR.

**`hack/refresh-current-state.sh:131` runs `go test ./... -count=1`
unisolated.** No `HOME` redirection, unlike `make test`. It fails on
daemon tests that read the real `~/.axis`, which blocks release prep on
any machine with an existing store. The Makefile already solves this;
the script should use the same pattern.

**`buildvcs` stamping fails inside a git worktree.** `make build`, `make
lint`, and `hack/refresh-current-state.sh` all fail with `error
obtaining VCS status: exit status 128` when run from a `git worktree`.
`-buildvcs=false` works around it. Worktrees are a normal Go workflow,
so this affects any contributor using one.

## Next

Merge, then `git tag v0.14.9 && git push origin v0.14.9`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: AXIS Contributor <axis-dev@example.invalid>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
toasterbook88 pushed a commit that referenced this pull request Aug 4, 2026
v0.14.9 was tagged and pushed, and the Release workflow failed at the Test Gate:
hack/verify-doc-facts.sh requires every git tag >= v0.7.0 to have a "## vX.Y.Z"
CHANGELOG header, and none existed. GoReleaser never ran, so no artifacts were
published for that tag.

Rather than move a pushed ref, this cuts v0.14.10 with the same contents and
leaves v0.14.9 in place. The gate enumerates git tags, not published releases,
so the orphaned tag needs a CHANGELOG entry permanently; it gets a short stub
recording that it was never published and pointing at v0.14.10.

  - CHANGELOG: v0.14.10 entry (contents of #270, #271, #272) + v0.14.9 stub
  - internal/buildinfo/version.go -> 0.14.10
  - docs/current-state.md regenerated
  - the three summary goldens embed the version string and move with it

Process note: there are two documentation gates, and release prep for v0.14.9
ran only verify-repo-truth.sh. Both verify-repo-truth.sh and verify-doc-facts.sh
must be green before tagging.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
toasterbook88 added a commit that referenced this pull request Aug 4, 2026
#274)

Cuts **v0.14.10**. Supersedes the `v0.14.9` tag, which was pushed but
never published.

## What happened

`v0.14.9` was tagged and the Release workflow **failed at the Test
Gate**:

```
verify-doc-facts: CHANGELOG.md missing entries for released tags: v0.14.9
```

`hack/verify-doc-facts.sh` requires every git tag `>= v0.7.0` to have a
`## vX.Y.Z` header. GoReleaser never ran — **no artifacts exist for that
tag**.

Release prep for v0.14.9 ran `verify-repo-truth.sh` and not
`verify-doc-facts.sh`. There are two documentation gates; both must be
green before tagging.

## Why v0.14.10 rather than moving the tag

Moving a pushed ref was the alternative. Cutting a new version touches
no existing ref. The gate enumerates **git tags**, not published
releases, so `v0.14.9` will require a CHANGELOG entry permanently — it
gets a short stub recording that it was never published and pointing
here.

## Changes

- `CHANGELOG.md` — `v0.14.10` entry covering #270, #271, #272, plus the
`v0.14.9` stub
- `internal/buildinfo/version.go` → `0.14.10`
- `docs/current-state.md` regenerated
- three summary goldens, which embed the version string

## Verification

```
verify-doc-facts    exit 0
verify-repo-truth   exit 0
make test           42 packages ok
make test-install   36 assertions, 0 failed
gofmt / vet         clean
```

## Next

Merge, then `git tag v0.14.10 && git push origin v0.14.10`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: AXIS Contributor <axis-dev@example.invalid>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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