fix: record the environment in the host owner record, and refuse a mismatch - #71
Merged
Conversation
…smatch
Two environments of one application could share a host, and the second would
adopt the first's containers and volumes rather than collide with them.
Every runtime name Onebox derives carries the application and not the
environment — the Compose project is the application name, containers are
<app>-<workload>-<n>, volumes are ob_<app>_<workload>_<volume> — because one box
runs one application. Environment.BasePath is a per-environment override, so the
release directories differ, but nothing else does.
Point staging at production's server and every check passes. The host owner
record names the application, and it matches. Preflight looks for foreign
resources and finds none, because the names it meets are its own. Then the
rollout takes over production's containers and mounts production's volumes.
Nothing in the system could see the difference, because the environment was
never written down anywhere the host could compare.
So it is written down now. The owner record becomes "<application>
<environment>", RequireHostOwner compares both halves, and a second environment
is refused with host_environment_mismatch before anything changes. Preflight
performs the same comparison, so a plan is not green for a deploy the engine
will reject.
A record claimed before this change carries the application alone. That still
identifies the owner, so it is honoured rather than refused — refusing would
strand every host claimed by an older ob. bootstrap upgrades it in place under
the host lock, which is also why the upgrade cannot use `set -C`: the file it is
replacing already exists. Until that runs, the application check applies exactly
as before.
The tests fail without the guard, which was checked by removing it and watching
them go red rather than assumed:
--- FAIL: TestRequireHostOwnerRefusesAnotherEnvironmentOfTheSameApplication
staging against a production-claimed host = <nil>, want HostEnvironmentMismatchError
`just check`, `golangci-lint run ./...` and `go test -race ./...` pass.
Closes #58.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`connect()` built engine.Options without Environment, so it was the empty
string. The engine derives every host path from it —
`names() = Spec.NamesFor(Opts.Environment)` — and an environment that is not in
the map falls back to the project default.
Reproduced against a project whose staging overrides base_path:
Environment="staging" -> AppDir /srv/staging/sample
Environment="" -> AppDir /var/lib/ob/sample
So `ob status --env staging`, `ob audit --env staging` and `ob logs --env
staging` have been reporting on /var/lib/ob/<app> while staging lives somewhere
else, and saying nothing about it. That predates this branch; it is the same
field, so it is fixed here rather than filed and left.
It was also a landmine under the ownership change. Those three commands are
read-only and never reach RequireHostOwner, so nothing breaks today — but the
next mutation added to that path would have compared an empty environment
against a recorded one and refused everything, naming an environment with no
name.
Found by reviewing this branch rather than by a test, because no test built an
engine the way cmd/ob does. There is one now.
`just check`, `golangci-lint run ./...` and `go test -race ./...` pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vishr
added a commit
that referenced
this pull request
Aug 18, 2026
fix: record the environment in the host owner record, and refuse a mismatch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #58 — the only open audit item that could lose data.
The bug
Two environments of one application could share a host, and the second would adopt the first's containers and volumes rather than collide with them.
Every runtime name carries the application and not the environment, because one box runs one application:
<app><app>-<workload>-<n>ob_<app>_<workload>_<volume>Environment.BasePathis a per-environment override, so release directories differ. Nothing else does.Point staging at production's server and every check passes. The owner record names the application, and it matches. Preflight hunts for foreign resources and finds none — the names it meets are its own. Then the rollout takes over production's containers and mounts production's volumes.
Nothing in the system could see the difference, because the environment was never written down anywhere the host could compare it.
The fix
The owner record becomes
<application> <environment>.RequireHostOwnercompares both halves and refuses a second environment with a new code,host_environment_mismatch, before anything changes.Preflight performs the same comparison, so a plan is never green for a deploy the engine will then reject.
Backward compatibility
A record claimed before this change carries the application alone. It still identifies the owner, so it is honoured rather than refused — refusing would strand every host claimed by an older
ob.bootstrapupgrades it in place under the host lock.That upgrade cannot use
set -C, unlike a first claim: the file it replaces already exists. It runs under the lock, having just re-read the record it is rewriting.Tests
Five new tests: the refusal, the same-environment acceptance, the legacy-record acceptance, the still-refused foreign application, and record round-tripping.
Non-vacuous — verified by removing the guard and watching them go red, not assumed:
Docs
errors.mdxregenerates with the new code. Two explanation pages updated by hand — the ownership boundary and the refusals list — because the interesting part is why two environments do not collide, which is the opposite of what a reader would assume.Verified
just check,golangci-lint run ./...,go test -race ./...all pass.