`validate_container_name`'s doc comment read "check it maps to a
hive-managed container". The body checked characters. Nothing in it
ever tested hive-managed-ness, so at the privsep boundary it asserted
a security property that was not there -- the kind of comment a future
reader relies on when deciding a name is already confined.
The confinement is real, it just lives elsewhere: all five callers
immediately apply `container_system_name`, so a caller naming
`hive-forge` reaches `h-hive-forge`, not the forge. That guarantee now
sits on `container_system_name`, which is the function that provides
it.
The `SIBLING_CONTAINERS` early-return in that validator was dead in
every path, not merely unused today: `validate_name_chars` accepts
`[a-z0-9-]`, so all three sibling names pass the fallthrough
identically -- both arms, same result. Infra containers reach the
lifecycle verbs through `ControlInfraContainer` and the
`InfraContainer` enum, which carries no free-form name at all, so no
sibling name reaches these functions. No test pinned the branch.
With the dead branch gone the function was character-for-character
`validate_agent_name`, so it is deleted and its five call sites use
that directly. `SIBLING_CONTAINERS` stays imported --
`validate_container_system_name` still needs it, and there the same
early-return IS load-bearing: without it an unprefixed sibling name
hits the bail.
No behaviour change. Gated: fmt, clippy -D warnings, 21 tests, rustdoc
under CI's docs-rustdoc lints (the two new intra-doc links resolve),
push-lints.
Follow-up to the same defect class as the `validate_container_name`
fix: prose on a privsep validator that does not match what the body
does. Found by enumerating all 33 fns in this file whose doc claims a
check or guarantee and reading body against doc. Three disagreed.
`validate_forge_admin_arg` overclaimed, which is the dangerous
direction. Its doc ended "Shell metacharacters are harmless since the
command is spawned directly (no shell), but we reject them
defensively" -- and no metacharacter is rejected anywhere: the body
takes null bytes, newlines and carriage returns only. The first clause
is true and is the actual safety argument, so it stays; the claim of a
defence that does not exist goes. The same doc also said "null bytes
and newlines" while the body rejects `\r` too.
`validate_bind_path` listed "no newlines, null bytes, or
double-quotes" and omitted the colon its body rejects -- the one that
matters most. The other three would corrupt the
`EXTRA_NSPAWN_FLAGS="..."` conf line; a colon corrupts nothing,
because `--bind=SRC:DST` is colon-separated, so a path carrying one
silently produces a different mount than the caller asked for. The
function's own bail! string already named colons.
Checked and left alone because they are accurate: `check_fd_agreement`
(both arms really do bail, so "in either direction" is earned),
`exec_forge_admin`, `validate_credential_name`.
Comments only, no behaviour change. Gated: fmt, clippy -D warnings, 21
tests, rustdoc under CI's docs-rustdoc lints, push-lints.
`docs/trust-boundary/boundary.md` § *the per-agent socket dir* already
explains why write permission on a directory confers the right to unlink
its entries and why the sticky bit does not restrain that. This comment
said the same thing in the same words, which is the "restating facts all
over" case rather than two independent explanations.
Measured rather than eyeballed, since "these look similar" is not a
finding: 6-gram intersection between this file's comments and that doc
went 36 -> 14. What remains is short shared phrasing — the
socket-activation sentence, which already carries its own pointer, and
the principals table, which is call-site material.
Kept at the call site, because a reader editing this line needs it and a
pointer alone would not stop the change:
* that 0751 is deliberate and security-relevant, with the consequence
named in one line;
* that `d` re-applies on every sync, so an external chown does not
survive the next agent's spawn.
That second one used to be written as history — a tug-of-war with a
`ChownSocketDir` unit that no longer exists. The constraint it carried is
still live, so it is stated in the present tense instead of as a story
about a removed mechanism.
Comments only; no behaviour change. Refs #3901.
Per mara's go-ahead on hyperhive#3902 ("getting started is good, but
terminal rendering does not go in there i think"):
Moved 21 top-level docs/*.md files into 7 new topic subdirectories
(existing web-ui/, turn-loop/, swarm/, tools/, crates/ untouched):
getting-started/ setup.md
agent-lifecycle/ agent-hierarchy.md, approvals.md, persistence.md
trust-boundary/ boundary.md, security.md
integrations/ forge.md, matrix.md, github.md, knowledge.md
networking/ gateway.md, network.md, snapshot-store.md
scheduler/ jobq.md, coordinator.md, ci.md, observability.md
process/ conventions.md, gotchas.md, pr-review-gate.md
web-ui/ terminal-rendering.md (moved into the EXISTING dir,
per mara's correction to the original getting-started
guess -- it's UI implementation detail, not onboarding)
The physical layout now matches docs/README.md's own topical headers,
which already amounted to this taxonomy -- see the scoping comment on
the issue for the two findings that motivated this (a genuine
duplication between CLAUDE.md's old "Reading paths" list and
docs/README.md's grouped one, since drifted out of sync with each
other; and the flat layout not matching the grouping we already had).
Fixed every cross-reference this moved across the whole repo (~120
files: docs/ internal links at every depth, Rust doc comments, nix
module option docs, crate READMEs) -- verified two ways: a grep sweep
confirming zero remaining references to any old path, and a script
that resolves every markdown link in docs/**/*.md + CLAUDE.md +
README.md against the filesystem and reports anything that doesn't
exist (zero broken links).
Collapsed CLAUDE.md's "Reading paths" section (the duplicate) down to
a pointer at docs/README.md, now the single index. Rewrote
docs/README.md itself to use the new subdirectory paths and added the
one doc it was missing that CLAUDE.md's old copy had (pr-review-gate.md).
Classified all 22 docs/*.md files first via a haiku subagent (mara's
suggestion) on two axes -- proposed grouping and operator-vs-
implementation focus -- before finalizing the taxonomy; spot-checked
the report and found internal inconsistencies (its classification
table disagreed with its own summary section for a few files), so this
taxonomy is my original proposal + the one correction mara gave
directly, not a blind application of the subagent's table. The
operator-focus data it gathered is still useful for a follow-up
content pass (docs skewing 'mixed' rather than pure operator-facing),
not addressed in this PR -- structure only.
nix fmt clean, both pre-push lints clean.
Was skipping live-log forwarding for the swap step on the reasoning
that it's near-instant in the happy case; mara pointed out a failure
there is exactly when the caller most wants the line in the build log,
not just a summary error. Factored the log+forward pattern into one
helper, used by both the nix-env --set and systemctl reload steps.
nixos-container update's own version-compat probe runs unconditionally
before --system-path is ever honored, dying on every agent's update.
Confirmed against nixos-container.pl's actual source: past that probe,
update's own action is just nix-env --set on the per-container profile,
then a systemctl reload if the container is running. Replicate that
directly instead of going through nixos-container update at all. create
is untouched.
The network-isolation doc comments carried prose docs/network.md
already owns, and three of them named `harness-base.nix` — a file
that does not exist. The `hyperhive-isolated-dns` oneshot lives in
nix/agent-modules/network.nix, which the doc gets right.
That is #3749's argument reproducing itself: the same fact written
in two places goes stale in the copy nobody reads. Linking removes
the class, not just the instance — a link cannot name a nonexistent
file without the doc noticing first.
Trap and measurement comments stay put, per the issue's scope: the
load-bearing HOST_ADDRESS default-route note, the unquoted
$EXTRA_NSPAWN_FLAGS expansion, and the "isolation is the only mode"
invariants are facts about this code, not about the subsystem.
The inline comment above write_bridge_dns_marker still said the marker
is 'written on isolate, removed otherwise, so the same shared container
toplevel behaves correctly in both modes'. There is one mode now.
Caught because argus pointed out that reading every changed function's
doc comment does not cover comments at the call sites -- the complete
form is to read every comment in the context around each hunk, which is
what git diff -U15 shows.
set_nspawn_flags said PRIVATE_NETWORK was forced 0 for web-UI
reachability; it is now unconditionally 1 and the UI is reached over the
bridge. write_bridge_dns_marker said it writes or removes the marker;
the removal path went with the host-netns case.
Neither was reachable by the sweeps that found the earlier ones: this
prose describes the *value in words* ('forced 0'), not the code's
literal PRIVATE_NETWORK=0, and neither names the env var. Found by
reading the doc comment of every function whose body this branch
changed.
Per mara on #3725: the on/off toggle is removed, and required env vars
unset lead to a crash. HIVE_NETWORK_ISOLATION is gone from
hive-network.nix -- it was the toggle.
Validation happens once at daemon startup rather than per container.
The variables are process-global, so a bad value breaks every container
rather than one: failing at boot gives a single diagnostic naming the
bad value, and cannot reach a state where some containers were
configured before it was noticed.
Option<NetworkIsolation> collapses to NetworkIsolation through the wire
type, client and helper, which deletes the branch instead of leaving it
unreachable. serde(default) is dropped on that field deliberately: a
request omitting isolation is now rejected rather than defaulting to a
container sharing the host's network namespace.
What this replaces was a silent security downgrade. Of the four ways
into the old fallback, two logged nothing at all -- a container came up
without isolation and the journal agreed it was fine.
Doc comments that still described the removed branch are updated
(argus's note on #3723 scoped that to this issue). The hive-priv one is
a minimal edit inside the block #3723 rewrites; de-splicing is that
PR's job.
Upstream's ExecStartPre re-registers only when .runner is absent, the
labels changed, or the registration token hash changed -- never when the
instance URL changed. So writing a fresh token and restarting the runner
registers only if the hash happens to differ, which is Forgejo's choice
to make: whether the admin registration-token endpoint mints a new token
per request or hands back a stable one is not ours to depend on.
hive-c0re already decides correctly -- ensure_ci_runner_registered only
reaches this helper once it has concluded the credentials are absent or
stale -- but the remediation was a no-op, so re-registration was
requested every boot and never happened.
Remove .runner before the restart so upstream takes its absence branch,
the one it evaluates unconditionally. NotFound is success; any other
error propagates rather than reporting Ok for a registration that never
ran.
An agent's config mount is a git repo, so it could read every branch and
the full history of a config whose currently deployed value is the only
thing it may act on -- and an abandoned branch looks no different from a
live one.
The knowledge bind already solved this with an empty tmpfs overlaid on
its .git. Same rule, extended: an agent is handed a working tree, never
a repository. Folds both cases into git_overlay_flags so the reason is
stated once instead of hardcoded per mount.
Config mounts are matched by shape rather than a name list because the
set grows at runtime with each child bound into a parent.
Per review: docs represent current state. Every "used to" / "no longer"
clause this branch introduced is gone — including the History section in
network.md, which was a whole subsection about a sync mechanism that
doesn't exist.
Where the removed clause was carrying a real constraint, the constraint
stays and is stated in the present tense instead of as a delta: nothing
narrows what the gateway's nginx can reach except the directory
permissions in front of a socket, and nothing bounds `ReloadGatewayNginx`
except the hard-coded unit name. Those read as rules now rather than as
the story of how they came to be rules.
The gateway's nginx + dnsmasq no longer run in their own nspawn container.
`nix/host-modules/hive-gateway/default.nix` loses the
`containers.hive-gateway` wrapper and everything that existed only to punch
holes in it: `privateNetwork = false`, `CAP_NET_ADMIN`, five bind mounts,
its own `stateVersion`, `networking.firewall.enable = false`,
`networking.resolvconf.enable = false`, and the `hive-gateway-resolv`
path+service pair. 465 -> 303 lines.
The container never bought isolation here. It shared the host netns by
necessity — nginx binds the host's :80/:443, dnsmasq answers on the bridge —
so each of those settings was undoing a boundary the gateway could not
afford in the first place.
Four things made it more than a deletion, none of them visible in the nix
diff:
- The self-signed cert service also imports the hive CA leaf, so removing it
with the container would have left nginx naming a missing cert file, which
it refuses to load at all.
- The nginx reload is a hive-priv verb. It still needs root, but no longer
for the reason its doc gave, and `--machine=` was both transport and
scope — so the unit name is now hard-coded in the helper as the
containment.
- The lifecycle verb named a container that stops existing.
- `journalctl -M hive-gateway` had no machine to enter.
Per the operator's ruling, the operator verb keeps working and agents lose
it. `InfraContainer` answered three questions that used to share an answer;
it now splits into `name()` (identity), `target()` (Container vs HostUnit),
`service_unit()` (the systemd unit), and `agent_restartable()`, which the
MCP restart path checks before the capability so the refusal cannot read as
"ask for infra_admin". `SIBLING_CONTAINERS` drops the gateway — it gates the
requests that name a container as a string — while `FromStr` still accepts
it, because that answers what a name is, not who may act on it. The
dashboard's gateway journal reads host journald filtered to `nginx.service`.
Prose was corrected where it only named a location, and re-argued where the
container was doing security work: a `0666` per-agent socket was safe
because only the gateway container had the directory bind-mounted. There is
no mount now, so the directory permissions are the whole of the access
control — the constraint holds, its mechanism doesn't.
Gate: nix fmt / clippy --all-targets -D warnings / cargo test all clean (710
tests); hivectl-cli.md regenerated from the clap tree. The nix eval was run
in both TLS shapes at this commit: every delta in the rendered
virtualHosts is one of the three intended path moves, dnsmasq settings are
byte-identical, and the absence probe flips true -> false with bindMounts
emptied.
container@.service sets Restart=on-failure and no start limit, so systemd's
defaults applied: 5 starts per 10s with RestartSec 100ms. That made the bound
a function of how fast a container dies -- one failing instantly tripped the
limit in under a second, one taking longer than ~2s never tripped it and
restarted forever.
The per-container drop-in already carried both sections this needs, so the
bound goes there rather than into a second file. StartLimit* are [Unit]
settings; systemd silently ignores them under [Service], which the pinned
body test now guards.
The values are hive-wide constants rather than wire fields: the bound is
policy, identical for every agent, and threading it through the socket as a
per-agent parameter would be plumbing for a value nobody varies.
Giving up is safe to make tight because it is not terminal -- reconcile
retries later, and the reset-failed already done before each start clears
the latch first.
argus caught a false negative in the shape rule, and it sits exactly in
the property the change is sold on -- "a new secret type is caught by
default".
`split_whitespace()` yields `"<token>,"` for a token with punctuation
glued to it, and the comma fails the alphabet check for the whole word,
so the line passes unredacted with the credential in it. `[<token>]`,
`"<token>"`, `(<token>)` and a no-whitespace-at-all blob all defeat it
the same way.
Whitespace is not what delimits a secret; the alphabet is. So scan the
line for a maximal run of >=32 alphabet characters and let punctuation
reset the counter. Simpler than the version it replaces, and it closes
the gap by construction rather than by enumerating the delimiters
someone might glue on next.
The existing tests all passed against the broken version because I wrote
them from the same mental model that produced the bug -- every fixture
had a space before the token. The new test carries the six shapes that
used to slip through.
`forgejo admin user generate-access-token` prints
Access token was successfully created: <40 hex>
and that line reached the host journal verbatim, for every agent
provisioned within journal retention. `read_host_journal` is a grantable
agent capability, so any agent holding it could read every other agent's
forge token and act fully as them.
The redactor missed it for a reason worth keeping. It matched the
substring "password", and its doc comment explains that choice: broad on
purpose, not pinned to forgejo's exact phrasing, so a *reworded* password
line still gets caught. That reasoning is sound and it guarded the wrong
axis -- the leak was a different KIND of secret on a differently worded
line. A denylist of one keyword fails open, and it failed open silently
while looking deliberate.
So there are now two independent rules, and the second matches on shape
rather than vocabulary: a whitespace-delimited run of >=32 characters
from the hex/base64url alphabet. A new secret type is caught by default
instead of by someone remembering to add a word.
It deliberately over-matches -- a nix store hash is also a long opaque
run and will redact its line. That is the correct direction to be wrong
in: a false positive costs one log line, a false negative costs a live
credential.
Two further sites, because fixing one of three is how these survive:
- stdout drops from INFO to DEBUG. On the success path that stream *is*
the product of the command (the freshly minted token) and nothing an
operator needs at default verbosity. Level and redaction are separate
layers; neither alone is sufficient.
- the failure path interpolated raw stderr into the `bail!` string, which
is propagated to the caller and logged. Redacting the log but not the
error leaves the same hole one step downstream.
`redact_password_line` is renamed to `redact_secret_line`. The old name
had become part of the problem: it read as "this line is safe" when it
only ever meant "this line has no password in it".
The regression test asserts its fixture contains no "password" before
asserting redaction -- otherwise it would pass under the old code and
prove nothing.
Rotating the already-exposed tokens is an operator action and is only
worth doing after this lands, or the new ones go into the journal too.
Both the gateway doc and the tmpfiles comment said "a directory without
the sticky bit lets any user unlink files in it". True of the old 0777,
but it names the wrong lever: write permission on a directory is what
confers the right to unlink its entries, and the sticky bit is only a
restraint on that -- it was never set here, so it is not what 0751
changes. Dropping o=w removes the permission outright.
The fix is unchanged; this is so a future reader doesn't go looking for
a sticky bit that was never there. Caught in review by argus.
/run/hive-agent/<name> was 0777 root root in steady state, not just during
first spawn. A directory without the sticky bit lets any user unlink files
in it, and the gateway container has all of /run/hive-agent bind-mounted
in, so anything that could reach the path could delete an agent's
agent.sock, bind its own, and receive that agent's todos from hive-c0re.
Two mechanisms were writing the dir and undoing each other: the tmpfiles.d
entry wrote 0777 root root, then hive-c0re round-tripped through hive-priv's
ChownSocketDir to narrow it. `d` re-asserts mode and owner on every apply
and the file is regenerated on any agent's spawn or destroy, so every such
event reset every agent's dir back to world-writable.
SyncAgentTmpfiles now carries each agent's container uid/gid and the entry
declares the answer: 0751 <uid> <gid>. Three principals need the dir and no
two share a group -- the harness binds its sockets (owner rwx), hive-c0re
dials agent.sock and the gateway's nginx dials web.sock (both only need
traverse, and both sockets are already 0666).
Deletes ChownSocketDir and ChmodSocketDir, both priv_client wrappers, the
either/or in host_config with its two swallowed warn!s, and the now-dead
socket_dir_path -- two verbs off the privileged helper's surface and one
round-trip off every agent spawn.
Also makes the two tmpfiles rules for /run/hive-agent itself agree: the
gateway module said hive-core, the generated file said root, and which won
depended on the order systemd read them in.
check_fd_agreement is the guard that keeps a descriptor and the request
it arrived with in agreement, and both of its rejections were untested.
An fd-taking op with no descriptor must not fall back to anything: a
temp file or the response socket would send an agent's state somewhere
the caller never asked for. The mirror case matters for a different
reason -- returning the error is what drops the OwnedFd and closes it,
so ignoring a stray descriptor instead would leak one per bad request
in a long-lived root process.
The third test pins both agreeing combinations, so the check is
rejecting mismatches rather than descriptors in general. Descriptors
are real /dev/null handles so the closing drop is genuinely exercised.
hive-priv read requests with BufReader::lines, which cannot surface
SCM_RIGHTS: ancillary data is attached to one specific recvmsg call, so
a buffered line reader takes the bytes and silently drops the
descriptor. Replace it with a recvmsg loop.
The pairing is deliberately trivial. hive-sock-client connects per
request, so a connection carries one line and at most one descriptor;
a second descriptor arriving before its line is a protocol error rather
than something to queue. check_fd_agreement rejects both mismatches --
an fd-taking op that got none, and a descriptor sent to an op that
takes none -- and dropping the OwnedFd on that path closes it.
recv_with_fds claims every descriptor the kernel attaches, including
ones this protocol never expects, because an fd we fail to claim leaks
for the life of the process. MSG_CMSG_CLOEXEC keeps a received
descriptor out of every btrfs and nixos-container child. The control
buffer is only cmsghdr-aligned, so descriptors are copied out
byte-wise instead of read through a more strictly aligned pointer.
SendAgentSnapshotToFd is SendAgentSnapshotToFile without the staging
file: same validation and -p parent handling, stdout wired to the
passed descriptor. It exists so hive-c0re can connect to a peer hive's
snapshot store, write the header itself, and hand over the connected
socket -- leaving this helper with no address, no protocol, and nobody
in the data path once the send starts.
Encoding "not configured" as weight 0 worked (the writer omitted the
line) but the type lied: 0 is not a legal cgroup v2 weight, and every
reader had to know the sentinel. Use Option<u32> end to end instead —
wire type, priv_client, HiveEnv, drop-in writer — so "unset" is a state
of the type rather than a magic value.
The nix options become nullOr, keeping their default of 80; null now
expresses "leave the setting out of the drop-in entirely" declaratively,
which is the useful shape on a host whose IO scheduler ignores
io.weight anyway.
Backward compat is unchanged: the fields stay #[serde(default)], so a
request from an older hive-c0re deserialises to None and reproduces the
pre-weights drop-in byte for byte. The test that pins that now passes
None instead of 0.
`CPUQuota=`/`MemoryMax=` are hard caps: they throttle an agent even when
the host is idle, so they are the wrong tool for "be polite under
contention". The cgroup v2 relative shares are, and neither was wired.
Adds `services.hyperhive.{agentCpuWeight,agentIoWeight}` (1..=10000,
default 80) threaded through the existing drop-in path: HiveEnv ->
write_dropins -> WriteResourceLimits -> hyperhive-limits.conf, next to
the caps already there. Hive-wide only, as the operator scoped it on the
issue: no per-agent override, no resource-limits.json field, no
dashboard form.
The default of 80 is below the kernel's 100, so agent containers yield
to everything *not* on this drop-in path -- host services and the infra
containers (hive-ci, hive-forge, hive-gateway, hive-matrix). It does not
rank agents against each other; they all carry the same weight.
`WriteResourceLimits` gains two `#[serde(default)]` fields, and the
writer treats weight 0 as "not configured" and omits the line, so an
older hive-c0re talking to a newer hive-priv still produces the exact
pre-weights drop-in. The body is extracted into `limits_dropin_body` so
that is covered by a test rather than asserted by eye.
`Coordinator::set_paused` wrote the marker directly with `std::fs::write`
from hive-c0re, which runs as the unprivileged `hive-core` user. The
agent's harness dir is chowned to the agent user on every container boot
(`user.nix`'s activation chown), mode 0755 — so hive-core can stat the
marker but gets EACCES creating or unlinking it. Pause therefore only
ever worked on an agent that had never booted; the read side works
because a stat needs traverse, not write, which is why the paused pill
and `is_paused` looked healthy.
Route both directions through hive-priv, the root helper that already
owns the other writes into agent-owned directories:
- `PrivRequest::SetAgentPaused { agent_name, paused }`, with the marker
filename constant moved to hive-priv-sock. That is the narrowest crate
all three sides share (hive-priv deliberately does not depend on
hive-sh4re, which re-exports it for the in-container resolver). A
private copy on any one side would break pause silently, since every
reader just sees "no marker".
- `write_agent_state_file` generalised to `write_agent_dir_file`, taking
the target directory: `state/` and `harness/` are both agent-owned,
which is the same reason both need root.
- resume unlinks via `remove_file`, which acts on the leaf and never
follows a symlink — an agent could otherwise plant a link at the
marker path and have root delete an arbitrary file.
`Coordinator::set_paused` becomes an async round-trip; its three call
sites were already async. Both directions stay idempotent because the
dashboard toggle and `hivectl pause|resume` fire without reading the
current state first.
`nixos-container stop` exiting 0 does not mean machined has dropped the
registration. A process sitting in the machine cgroup without being a
child of the container's init never receives the shutdown's SIGTERM if
it has been SIGSTOP'd, so the registration outlives the "successful"
stop. Every later start then fails with "Failed to register machine:
already exists", and machined re-persists the stale record across its
own restart, so there is no cleaning it up afterwards.
StopContainer now asks for the stop, waits for machined to release the
name, escalates to SIGKILL if it hasn't, re-verifies, and fails loudly
if the name is still held — so a caller is never told the stop worked
and then walks into the confusing registration error.
The probe resolves the name through machined's GetMachine, the same
lookup that later rejects the registration, rather than checking the
container's systemd unit: the unit going inactive while the name is
still held is precisely the case being caught.
Verify-and-escalate lives in the helper, not at a call site, so every
stop gets it — dashboard, reconcile, destroy, cold-start fallback. The
start path already distrusts its own exit code the same way; this is
the missing half of that pair.
Both groups only ever act on a single managed agent's state dir, so
they belong in the `agents` namespace rather than as top-level verbs
next to `forge` / `matrix` / `wg`.
Renames `quota limit` -> `quota set`: the enclosing group already
carries the noun, so the bare verb matches the flat `set-parent` /
`set-limits` spelling without stuttering, and it removes the
`set-limits` (cpu/mem) vs `quota limit` (disk) ambiguity. Adds a
cross-pointer from `set-limits` to `agents quota`.
Handlers stay in their own modules; `run_agents` gains the reparenting
glue. Regenerates docs/tools/hivectl-cli.md.
Refs #2724
`container_run` logs every stdout line at INFO (target `nixos-container`)
as operation progress. For the read-only `list` op — called on the hot
path (dashboard rescan, forge + boot sweeps) — that stdout is the return
value, not progress, so every call logs the full ~28-line container roster
at INFO. hive-c0re calls it several times a second, flooding the host
journal.
Gate the stdout per-line INFO logging on the op not being `list`. Mutating
ops still log their progress; stderr is still logged for every op (errors
matter regardless). No behaviour change beyond log volume.
Second increment of the per-crate README effort, covering the rest of the
infra/wire/priv column: hive-priv, hive-metric, hive-types, hive-sh4re,
hive-core-agent-sock, hive-agent-sock. Same shape as the first batch —
purpose + when-to-use, and point at the crate-root //! docs plus the
relevant docs/ pages rather than duplicating them. Wires
readme = "README.md" into each Cargo.toml [package].
Disjoint from the batch-1 crates, so the two increments compose cleanly.
Part A of moving hive-ci runner registration off the boot-critical path.
Adds a root-side hive-priv op that writes the runner registration token to
the host env-file /run/hive-ci/runner-token (in-place, preserving the inode
the container bind-mounts) and restarts the in-container gitea-runner-hive
unit. The forge admin token stays in hive-c0re; only the registration token
reaches the host env-file the container mounts read-only. The c0re-side
caller (ensure_ci_runner_registered) + the nix boot-path change land next on
this branch.
Split the priv-socket wire types (PrivRequest/PrivResponse/PrivEvent and
friends) out of hive-sh4re into their own hive-priv-sock crate, mirroring
the existing hive-host-sock split. hive-priv — the root-privileged
helper — now depends on just this narrow protocol crate instead of the
much larger daemon-shared crate, shrinking its dependency surface and
making the privsep boundary easier to audit. No server/client
implementation lives here, only the wire contract; hive-c0re still
depends on hive-sh4re directly for everything else.
- regenerate docs/tools/hivectl-cli.md for the new `subvol snapshot send` verb
- close the TOCTOU on the no-overwrite guard: File::options().create_new(true)
(O_CREAT|O_EXCL) instead of exists()-then-create, so the guarantee is
atomic against a concurrent request racing the same dest filename
- warn (not silently swallow) if cleaning up a partial export after a
failed btrfs send itself fails, so a stuck garbage file masquerading
as a completed export is visible in the log
SendAgentSnapshotToFile priv op: btrfs send [-p <parent>] <snapshot> to a
file under MIGRATE_STAGING_ROOT. Standalone-useful as a point-in-time
snapshot export/backup today; the cross-hive ssh-piped leg (auth/trust
design posted on #1763, awaiting mara/damocles steer) is a later,
separate piece this doesn't block on.
- hive-sh4re: PrivRequest::SendAgentSnapshotToFile + MIGRATE_STAGING_ROOT
- hive-priv: validates names, refuses to overwrite an existing export,
cleans up a partial file on btrfs send failure
- hive-c0re: priv_client::send_agent_snapshot_to_file
- hivectl: `hivectl subvol snapshot send <agent> <label> [--parent <label>] --dest <file>`
Per mara: "i would have even disallowed ., we are making up the rules
here lets go strict". validate_credential_name now restricts to
[A-Za-z0-9_-] (no dot at all) instead of [A-Za-z0-9_.-] + a separate
".." substring check — simpler rule, and there's no legitimate need
for a dot in either a systemd credential id or a hive- prefixed
snapshot label. Matching hivectl client-side check + wire-proto doc
comments updated.
Per mara's review: validate_credential_name allowed any [A-Za-z0-9_.-]
byte sequence, which permits a literal ".." substring. Not currently
exploitable (snapshot_path() embeds the label inside a single
format!()'d path component with no "/" in the allowed charset, so
there's no directory to traverse into), but it's a landmine for any
future caller that builds a path via PathBuf::from(name) directly
instead of the current string-embedding. Reject ".." outright in the
shared validator, plus a matching client-side check in hivectl for
fail-fast UX (hive-priv's copy is still the authoritative one).
