host: announce the repairs, refuse a drop rule as a gate, and never overwrite an operator's db_path
Review fixes on the grant-drift pass, plus the migration door the review
found missing.
I1 — a pass that writes no bytes still repairs UNIT STATE, and it did so in
total silence: the announcement was gated on res.Changed, so an operator who
stopped carlos-app@keymail.dev by hand to debug it got no log line at all
when the hourly pass started it again an hour later. That is the single most
surprising thing this pass does. Every drift repair now announces itself with
the verb and the reason ("the service was not running and this route declares
daemon — started it"), and only on success — a failed verb is already
reported by the error line, and "started it" is not a claim to make about a
start that did not happen.
I2 — parseNftSkuids counted uids out of DROP rules, so `meta skuid 998 drop`
read as an allowlist entry and the advisory went quiet about exactly the uid
the firewall blocks. Fail-open, on the one check whose failure mode is the
silent starvation of F1. A rule is now read only when its verdict accepts,
and the table pins harden-host.sh's real output shape (inline numeric
`ip daddr 169.254.169.254 meta skuid <n> accept`, no named sets) alongside
the hostile drop form.
I3 — the db_path stamp could silently kill replication. A pre-migration
route's operator-set -db names where the data lives RIGHT NOW (/data/<host>.db,
the file litestream is streaming today); overwriting it with the
post-migration path before the §5 copy has happened points the replicator at
a file that does not exist. The stamp now only replaces a non-empty value it
recognises as its own — under this box's state dir for this host — and
advises otherwise; the log names the value it replaced; and SetDBPath gains
SetCaps's compare-and-swap shape, so an hourly pass cannot clobber a value
written underneath it.
RULING (orchestrator, overruling the plan): the stamp takes dbs[0] AS
DECLARED, not sorted. An author writing ["console.db", "audit.db"] means
console.db is the one that matters, and lexicographic order would have
replicated audit.db instead — the platform quietly choosing a different
database than the app named. Declaration order is the app's own statement of
which database is primary; nothing else in the manifest carries it.
RULING (ungated boxes): a readable ruleset with no IMDS gate in it is a third
fact with a third fix, not "could not be read". A host_role app on an ungated
box is F1's exposure with nothing in front of it, so the line stays hourly and
now names the fix — run platform-infrastructure scripts/harden-host.sh on
this box, then add the uid.
NEW — `carlos route -backing <unit|"">` (registry.SetBacking). The migration
door design §5b/§5c reference and the code did not have: backing was
choosable at `carlos add` and never afterwards, so the cutover's stamp step
had no command and the reversal had none either. Same rails as the other
single-route axes — requires -host, exclusive with -grant/-revoke/-hibernate/
-channel/-environment, value checked before the registry is opened — plus
two of its own in backingRail: every Platform row (stricter than the other
axes, because a second unit under an alias would claim ports the real route
holds), and any kind other than instance.
Minors: the exec-backed unreadable-manifest line no longer claims drop-ins
are being left alone (it has none); the candidate-set comment says it is
stricter than warnBackingMismatch's rather than identical; and capabilityPass
says why discarding Converge's (Changed, err) is right for THIS caller —
moved bytes here are the route's own live shape, the steady state this pass
converges toward, so the next pass's diff self-heals and a Restore would undo
a correct repair.
Mutation-checked: gating the stamp on converge success, reverting the repair
announcement, dropping the accept-verdict check, and sorting dbs each fail a
named test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
9 files changed,
+637
−71
cli.go+105 −8cli_test.go+58 −0internal/caps/converge.go+33 −4internal/host/adopt_advice.go+61 −24internal/host/adopt_advice_test.go+32 −0internal/host/caps_pass.go+74 −22internal/host/caps_pass_test.go+171 −9internal/registry/registry.go+52 −2internal/registry/registry_test.go+51 −2
diff --git a/cli.go b/cli.go| index 1086d1948..985db217d 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -305,6 +305,7 @@ func runRoute(args []string) { |
| grant := fs.String("grant", "", "capability token to grant this route ON THIS BOX: port:<n> (privileged, never 22/80/443) | host-role") |
| revoke := fs.String("revoke", "", "capability token to revoke from this route on this box") |
| hibernate := fs.Bool("hibernate", false, "hibernate this route when idle; -hibernate=false stops it hibernating") |
| + backing := fs.String("backing", "", `who owns this route's process: "unit" (a carlos-app@<host> systemd unit) | "" (an exec child of the agent)`) |
| allowVersionChange := fs.Bool("allow-version-change", false, |
| "allow repointing to a channel that RESOLVES but holds a different version than the route currently serves "+ |
| "(never overrides an unresolvable target, a bad signature, or a missing pubkey)") |
| @@ -316,17 +317,24 @@ func runRoute(args []string) { |
| // fs.Visit reports only the flags actually present on the command line, |
| // which keeps the operator's syntax the natural `-hibernate` / |
| // `-hibernate=false` rather than a quoted tri-state string. |
| - hibernateSet := false |
| + // -backing is tri-state for the same reason, and more sharply: "" is a |
| + // LEGAL VALUE of that column (an exec child), so a flag left unmentioned |
| + // and a flag set to "" cannot be told apart by the value alone. |
| + hibernateSet, backingSet := false, false |
| fs.Visit(func(f *flag.Flag) { |
| - if f.Name == "hibernate" { |
| + switch f.Name { |
| + case "hibernate": |
| hibernateSet = true |
| + case "backing": |
| + backingSet = true |
| } |
| }) |
| const usage = "usage: carlos route -host <host> [-channel <channel>] [-environment <name>] [-registry <db>] [-allow-version-change] [-dry-run]\n" + |
| " or: carlos route -app <app> [-channel <channel>] [-environment <name>] [-account <sqid|name|all>] [-registry <db>] [-allow-version-change] [-dry-run]\n" + |
| " or: carlos route -host <host> (-grant | -revoke) <port:n|host-role> [-registry <db>] [-dry-run]\n" + |
| - " or: carlos route -host <host> -hibernate[=false] [-registry <db>] [-dry-run]" |
| + " or: carlos route -host <host> -hibernate[=false] [-registry <db>] [-dry-run]\n" + |
| + ` or: carlos route -host <host> -backing <unit|""> [-registry <db>] [-dry-run]` |
| if *host == "" && *app == "" { |
| log.Fatal(usage) |
| } |
| @@ -362,8 +370,8 @@ func runRoute(args []string) { |
| log.Fatalf("route: %v", err) |
| } |
| } |
| - if *channel == "" && *environment == "" && !grantAxis && !hibernateSet { |
| - log.Fatal(`route: at least one of -channel, -environment, -grant, -revoke or -hibernate is required. "" only means stable-by-default at route CREATION (carlos add) — ` + |
| + if *channel == "" && *environment == "" && !grantAxis && !hibernateSet && !backingSet { |
| + log.Fatal(`route: at least one of -channel, -environment, -grant, -revoke, -hibernate or -backing is required. "" only means stable-by-default at route CREATION (carlos add) — ` + |
| "this verb sets an explicit binding, and needs an explicit target to set it to.") |
| } |
| if *channel != "" && !release.ValidChannel(*channel) { |
| @@ -408,7 +416,7 @@ func runRoute(args []string) { |
| // The single-route axes short-circuit here, before the channel/environment |
| // machinery below: they write one column of one row, through one narrow |
| // setter, and share none of that batch's all-or-nothing plumbing. |
| - if grantAxis || hibernateSet { |
| + if grantAxis || hibernateSet || backingSet { |
| rt, err := r.Get(*host) |
| if err != nil { |
| log.Fatal(err) |
| @@ -416,10 +424,13 @@ func runRoute(args []string) { |
| if rt == nil { |
| log.Fatalf("route: no route for host %q", *host) |
| } |
| - if grantAxis { |
| + switch { |
| + case grantAxis: |
| runRouteGrant(r, *rt, *grant, *revoke, *dryRun) |
| - } else { |
| + case hibernateSet: |
| runRouteHibernate(r, *rt, *hibernate, *dryRun) |
| + default: |
| + runRouteBacking(r, *rt, *backing, *dryRun) |
| } |
| return |
| } |
| @@ -959,6 +970,92 @@ func runRouteHibernate(r *registry.Registry, rt registry.Route, on, dryRun bool) |
| fmt.Printf("%s%s: hibernate %v -> %v\n", prefix, rt.Host, rt.Hibernate, on) |
| } |
| +// runRouteBacking stamps who owns a route's process — the door design §5b |
| +// and §5c reference and the code did not have: backing was choosable at |
| +// `carlos add` and never afterwards, so the migration's stamp step (stop the |
| +// hand-provisioned unit, copy the data, stamp the row, let the next adopt |
| +// pass converge its capabilities) had no command, and neither did the |
| +// reverse move the migration's reversibility depends on. |
| +// |
| +// Three rails, all structural, none overridable: |
| +// |
| +// - a Platform route is refused. A platform ALIAS materializes nothing by |
| +// design (a second carlos-app@ unit claiming :25 would break the real |
| +// one), and the domains reconciler rebuilds these rows from mintedRoute |
| +// — which sets backing from its own derivation, so a stamp here is |
| +// reverted on the next pass. Same refusal, same reason, as the other |
| +// single-route axes. |
| +// - kind != "instance" is refused: only an instance has a process for |
| +// anything to back. A service or static route has no ExecStart to move. |
| +// - the value is "" or BackingUnit, checked at the flag before the |
| +// registry is even opened. |
| +// |
| +// It does NOT check whether the data has been copied, or whether a unit |
| +// exists — it cannot, and pretending to would be worse than not trying. The |
| +// migration checklist is the operator's; this verb is the one write in it, |
| +// and it prints what the next adopt pass will do with the new value so the |
| +// operator can decide whether they are ready for it. |
| +func runRouteBacking(r *registry.Registry, rt registry.Route, backing string, dryRun bool) { |
| + if err := backingRail(rt); err != nil { |
| + log.Fatalf("route: %s: %v", rt.Host, err) |
| + } |
| + prefix := "" |
| + if dryRun { |
| + prefix = "(dry run) " |
| + } |
| + if rt.Backing == backing { |
| + fmt.Printf("%s: backing already %s — nothing to do\n", rt.Host, backingWord(backing)) |
| + return |
| + } |
| + if !dryRun { |
| + if err := r.SetBacking(rt.Host, backing); err != nil { |
| + log.Fatalf("route: write: %v", err) |
| + } |
| + } |
| + fmt.Printf("%s%s: backing %s -> %s\n", prefix, rt.Host, backingWord(rt.Backing), backingWord(backing)) |
| + // The consequence, named at the moment of typing rather than discovered |
| + // in an hourly log: the two halves of the platform that branch on this |
| + // column change sides together. |
| + if backing == registry.BackingUnit { |
| + fmt.Printf(" the next adopt pass converges this route's capability drop-ins and config sync writes /etc/carlos/apps/%s.env; the agent stops materializing /data/%s.config.env\n", rt.Host, rt.Host) |
| + } else { |
| + fmt.Printf(" the next adopt pass stops converging this route's capabilities (any drop-ins already written are left exactly as they are, for an operator to remove) and config delivery returns to the agent's /data/%s.config.env\n", rt.Host) |
| + } |
| +} |
| + |
| +// backingRail is the row-shape half of runRouteBacking's refusals, split out |
| +// as a pure function for the same reason grantRail is: a rail nobody can |
| +// exercise without running the process is a rail nobody tests. |
| +// |
| +// Refusing EVERY Platform row (not only Platform+Canary, as the channel and |
| +// environment axes do) is deliberate and stricter. Those axes refuse the |
| +// canary-minted rows because the reconciler re-derives them; this one refuses |
| +// a plain platform apex too, because backing is not merely rebuilt for those |
| +// rows — it is meaningless. A platform alias is a second name for one |
| +// process, and a second carlos-app@ unit under it would claim ports and a |
| +// state directory the real route already owns. |
| +func backingRail(rt registry.Route) error { |
| + if rt.Platform { |
| + return fmt.Errorf("Platform route — a platform alias is a second name for one process, so a unit of its own would " + |
| + "claim ports and a state directory the real route already holds; and the domains reconciler rebuilds these rows " + |
| + "from values it derives itself. This verb does not own this row") |
| + } |
| + if rt.Kind != "instance" { |
| + return fmt.Errorf("backing is only meaningful for an instance route (kind %q) — a %s route has no process of its own to back", rt.Kind, rt.Kind) |
| + } |
| + return nil |
| +} |
| + |
| +// backingWord renders a backing column value for an operator — the empty |
| +// string is a real value of this column, and printing it as "" reads as a |
| +// bug rather than as an answer. |
| +func backingWord(backing string) string { |
| + if backing == "" { |
| + return `"" (exec child of the agent)` |
| + } |
| + return backing |
| +} |
| + |
| // daemonHibernateRefusal is the one sentence both doors say — `carlos route |
| // -hibernate` and `carlos add -hibernate` — so an operator who meets it at |
| // one of them recognises it at the other. |
diff --git a/cli_test.go b/cli_test.go| index f0f06410a..69fe99115 100644 |
| --- a/cli_test.go |
| +++ b/cli_test.go |
| @@ -374,3 +374,61 @@ func TestRouteDaemonCheck(t *testing.T) { |
| t.Fatal("a pointer signed by another key must not prove anything") |
| } |
| } |
| + |
| +// TestBackingRailRefusals: `carlos route -backing` is the migration door |
| +// (app-capabilities design §5b/§5c), and its two row-shape refusals are |
| +// structural — no flag overrides either. It is STRICTER than the channel and |
| +// environment axes on platform rows: those refuse only Platform+Canary |
| +// (rebuilt by the reconciler), this refuses a plain platform apex too, |
| +// because a second carlos-app@ unit under an alias would claim ports and a |
| +// state directory the real route already holds. |
| +func TestBackingRailRefusals(t *testing.T) { |
| + instance := registry.Route{Host: "console.example.com", Kind: "instance", App: "console"} |
| + cases := []struct { |
| + name string |
| + rt registry.Route |
| + says string |
| + }{ |
| + {"platform apex", func() registry.Route { |
| + rt := instance |
| + rt.Platform = true |
| + return rt |
| + }(), "does not own"}, |
| + {"platform+canary row", func() registry.Route { |
| + rt := instance |
| + rt.Platform, rt.Canary = true, true |
| + return rt |
| + }(), "does not own"}, |
| + {"service route", func() registry.Route { |
| + rt := instance |
| + rt.Kind = "service" |
| + return rt |
| + }(), "only meaningful for an instance route"}, |
| + {"static route", func() registry.Route { |
| + rt := instance |
| + rt.Kind = "static" |
| + return rt |
| + }(), "only meaningful for an instance route"}, |
| + } |
| + for _, c := range cases { |
| + t.Run(c.name, func(t *testing.T) { |
| + err := backingRail(c.rt) |
| + if err == nil { |
| + t.Fatalf("backingRail(%+v) accepted; want a refusal", c.rt) |
| + } |
| + if !strings.Contains(err.Error(), c.says) { |
| + t.Fatalf("refusal %q does not say %q", err, c.says) |
| + } |
| + }) |
| + } |
| + // Both directions of the migration are the same verb on the same row, so |
| + // both must clear the rail from either starting value. |
| + if err := backingRail(instance); err != nil { |
| + t.Fatalf("backingRail(exec-backed instance) = %v, want accepted", err) |
| + } |
| + unit := instance |
| + unit.Backing = registry.BackingUnit |
| + if err := backingRail(unit); err != nil { |
| + t.Fatalf("backingRail(unit-backed instance) = %v, want accepted", err) |
| + } |
| +} |
diff --git a/internal/caps/converge.go b/internal/caps/converge.go| index c185e0b55..b931cbb0e 100644 |
| --- a/internal/caps/converge.go |
| +++ b/internal/caps/converge.go |
| @@ -421,6 +421,27 @@ type unitState struct { |
| err error |
| } |
| +// repaired announces a drift repair — a verb this pass issued because |
| +// systemd's RUNNING state disagreed with what the route declares, not because |
| +// any byte changed. |
| +// |
| +// It is deliberately not folded into Converge's trailing "converged" line, |
| +// which is gated on res.Changed: the repairs below fire on passes where |
| +// nothing was written at all, and reporting them there made the most |
| +// surprising thing this pass does — starting a unit an operator stopped by |
| +// hand ten minutes ago to debug it — completely silent. Whoever stopped that |
| +// unit needs the hourly log to say which verb ran and why. |
| +// |
| +// Only a repair that SUCCEEDED is announced; a failed one is already reported |
| +// by applyUnitState's error line, and "started it" is not a claim to make |
| +// about a start that did not. |
| +func (o Ops) announceRepair(host string, err error, format string, a ...any) { |
| + if err != nil { |
| + return |
| + } |
| + o.logf(" %s: %s", host, fmt.Sprintf(format, a...)) |
| +} |
| + |
| // applyUnitState is steps 7-9: the socket restart ordering, the enable/start |
| // a daemon route's drift repair needs, and the service restart. Every verb |
| // here runs AFTER Converge's daemon-reload, never before it. |
| @@ -467,20 +488,26 @@ func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unit |
| // nothing" true for a daemon route, not just for a quiet one). |
| if sockLive { |
| if !o.query("is-enabled", sockUnit) { |
| - note(o.systemctl("enable", sockUnit)) |
| + err := o.systemctl("enable", sockUnit) |
| + note(err) |
| + o.announceRepair(p.Host, err, "%s was not enabled — enabled it, or its ListenStream binds nothing after a reboot", sockUnit) |
| } |
| if !o.query("is-active", sockUnit) && !restartedService { |
| // Bringing an inactive socket up goes through the same ordering: |
| // the service may be holding the port itself today (keymail binds |
| // :25 until its LISTEN_FDS change lands), and a bare socket start |
| // under it is the EADDRINUSE race again. |
| - note(o.socketSequence(p.Host, p.Set.Daemon, p.DeferRestart, "start")) |
| + err := o.socketSequence(p.Host, p.Set.Daemon, p.DeferRestart, "start") |
| + note(err) |
| + o.announceRepair(p.Host, err, "%s was not active — started it through the stop → socket → start ordering, so nothing races the service for the port", sockUnit) |
| restartedService = true |
| } |
| } |
| if p.Set.Daemon { |
| if !o.query("is-enabled", svcUnit) { |
| - note(o.systemctl("enable", svcUnit)) |
| + err := o.systemctl("enable", svcUnit) |
| + note(err) |
| + o.announceRepair(p.Host, err, "%s was not enabled — enabled it, so it comes back after a reboot", svcUnit) |
| } |
| if !o.query("is-active", svcUnit) && !p.DeferRestart { |
| // enable+start, not try-restart: a daemon that crashed into a |
| @@ -490,7 +517,9 @@ func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unit |
| // Deferred, this is the start that would run the OLD binary: the |
| // service is inactive here precisely because step 7 just stopped |
| // it, and the caller's Restart is one symlink swap away. |
| - note(o.systemctl("start", svcUnit)) |
| + err := o.systemctl("start", svcUnit) |
| + note(err) |
| + o.announceRepair(p.Host, err, "the service was not running and this route declares daemon — started it") |
| restartedService = true |
| } |
| } |
diff --git a/internal/host/adopt_advice.go b/internal/host/adopt_advice.go| index 6c3d9d090..dea7fa9df 100644 |
| --- a/internal/host/adopt_advice.go |
| +++ b/internal/host/adopt_advice.go |
| @@ -5,7 +5,6 @@ |
| package host |
| import ( |
| - "bytes" |
| "log" |
| "os/exec" |
| "os/user" |
| @@ -112,10 +111,13 @@ const imdsAddr = "169.254.169.254" |
| // means nothing is running under it yet and there is nothing for the gate |
| // to be missing. Advising then would be an hourly line about a uid that |
| // does not exist. |
| -// - gateUIDs answers (uids, ok). ok == false is "I could not read a gate", |
| -// which is NOT the same claim as "the uid is absent from the gate" and |
| -// gets its own line, because acting on the wrong one of those two sends |
| -// an operator to edit a firewall that is fine. |
| +// - gateUIDs answers (uids, ok), and three different things can be true. |
| +// ok == false is "I could not read a gate". A readable ruleset with NO |
| +// IMDS gate in it at all is a third claim again — the box is ungated, and |
| +// the fix is not "add a uid" but "harden this box first" (tito has no |
| +// gate today). Each gets its own line, because acting on the wrong one of |
| +// the three sends an operator to edit a firewall that is fine, or to add |
| +// a uid to a gate that does not exist. |
| // |
| // A route whose grants column does not parse is skipped in silence here: |
| // buildPlans already says so once, naming the row and the fix, and repeating |
| @@ -147,7 +149,11 @@ func adviseHostRoleGateMissing(routes []registry.Route, uidOf func(user string) |
| continue |
| } |
| if !ok { |
| - log.Printf("adopt: %s holds the host_role grant on %s but this box's IMDS gate could not be read (nft absent, unreadable, or carrying no rule naming %s) — confirm by hand that the uid is allowed (see the app-capabilities design §4b)", u, rt.Host, imdsAddr) |
| + log.Printf("adopt: %s holds the host_role grant on %s but this box's nft ruleset could not be read (nft absent, or this pass is not root) — confirm by hand that uid %d is allowed to reach %s (see the app-capabilities design §4b)", u, rt.Host, uid, imdsAddr) |
| + continue |
| + } |
| + if len(uids) == 0 { |
| + log.Printf("adopt: %s (uid %d) holds the host_role grant on %s but no IMDS uid gate found in the nft ruleset — this box does not gate %s at all, so every process on it can reach the instance role; run platform-infrastructure scripts/harden-host.sh on this box, then add uid %d", u, uid, rt.Host, imdsAddr, uid) |
| continue |
| } |
| if !uids[uid] { |
| @@ -178,34 +184,45 @@ func lookupUID(name string) (int, bool) { |
| // the parse never has to resolve a printed account NAME back to a number and |
| // can never mistake one for the other. |
| // |
| -// ok is false when nft cannot run (absent binary, not root, no ruleset) AND |
| -// when the ruleset carries no rule naming the metadata address at all — a box |
| -// with no IMDS gate is one this advisory cannot make the "uid is missing" |
| -// claim about, and saying "could not read a gate" of it is the honest answer |
| -// rather than telling an operator to add a rule to a firewall that gates |
| -// nothing. |
| +// ok is false ONLY when the ruleset could not be read at all (absent binary, |
| +// not root, nft error). A ruleset that reads fine and carries no IMDS gate is |
| +// (empty, true) — a different fact, with a different fix, and one this |
| +// advisory must not go quiet about: a host_role app on a box with no IMDS |
| +// gate is not safe, it is ungated, which is F1's exposure with nothing in |
| +// front of it. |
| func nftGateUIDs() (map[int]bool, bool) { |
| out, err := exec.Command("nft", "-n", "list", "ruleset").Output() |
| - if err != nil || !bytes.Contains(out, []byte(imdsAddr)) { |
| + if err != nil { |
| return nil, false |
| } |
| return parseNftSkuids(out), true |
| } |
| -// parseNftSkuids reads the uids allowed to reach the metadata address out of |
| +// parseNftSkuids reads the uids ALLOWED to reach the metadata address out of |
| // `nft -n list ruleset` output. |
| // |
| -// Scoped to lines naming imdsAddr on purpose: a box has other skuid rules |
| -// (SSH, a backup agent), and counting those would report a gate that allows |
| -// uids it does not allow — the fail-OPEN direction, where this advisory goes |
| -// quiet about a starvation it should be shouting about. |
| +// The production gate is inline and numeric — `ip daddr 169.254.169.254 meta |
| +// skuid <n> accept`, one rule per allowed uid, written by the infra repo's |
| +// scripts/harden-host.sh, with no named sets today — so that is the shape |
| +// this reads, plus nft's `skuid { a, b }` set spelling for free. |
| // |
| -// Both spellings nft prints are read: `skuid 998` and the set form |
| -// `skuid { 998, 999 }`. Negated and range matches (`skuid != 0`, |
| -// `skuid 100-200`) are deliberately not interpreted — neither is an |
| -// allowlist of one uid, and guessing at one would be the fail-open direction |
| -// again; an unparsed rule simply leaves the uid looking absent, which at |
| -// worst produces one advisory line an operator can dismiss. |
| +// Two rails, both in the fail-CLOSED direction, because every mistake this |
| +// function can make ends in an advisory that stays quiet about a silent |
| +// starvation: |
| +// |
| +// - scoped to lines naming imdsAddr: a box has other skuid rules (SSH, a |
| +// backup agent) and counting those would report a gate allowing uids it |
| +// does not. |
| +// - scoped to lines whose VERDICT accepts. `... meta skuid 998 drop` is the |
| +// opposite of an allowlist entry, and reading it as one would report the |
| +// gate as open for exactly the uid it blocks. A jump/goto/return is |
| +// counted as accepting: it hands the packet to another chain rather than |
| +// dropping it here, and over-reporting a uid as gated is the direction |
| +// that costs one dismissed advisory line rather than a silent outage. |
| +// |
| +// Negated and range matches (`skuid != 0`, `skuid 100-200`) are deliberately |
| +// not interpreted: neither is an allowlist of one uid, and guessing would be |
| +// the fail-open direction again. |
| func parseNftSkuids(out []byte) map[int]bool { |
| uids := map[int]bool{} |
| for _, line := range strings.Split(string(out), "\n") { |
| @@ -213,6 +230,9 @@ func parseNftSkuids(out []byte) map[int]bool { |
| continue |
| } |
| fields := strings.Fields(line) |
| + if !acceptingVerdict(fields) { |
| + continue |
| + } |
| for i := 0; i < len(fields); i++ { |
| if fields[i] != "skuid" || i+1 >= len(fields) { |
| continue |
| @@ -232,6 +252,23 @@ func parseNftSkuids(out []byte) map[int]bool { |
| return uids |
| } |
| +// acceptingVerdict reports whether a rule's verdict lets the packet through. |
| +// A rule that drops or rejects is never read for allowed uids, whatever else |
| +// it says — and a rule with no verdict at all (a counter, a comment) allows |
| +// nothing either. |
| +func acceptingVerdict(fields []string) bool { |
| + accepting := false |
| + for _, f := range fields { |
| + switch strings.Trim(f, `",;`) { |
| + case "drop", "reject": |
| + return false |
| + case "accept", "return", "goto", "jump": |
| + accepting = true |
| + } |
| + } |
| + return accepting |
| +} |
| + |
| // addUID records one uid token, tolerating the punctuation nft's set syntax |
| // carries ("998," inside braces) and ignoring anything that is not a plain |
| // non-negative number. |
diff --git a/internal/host/adopt_advice_test.go b/internal/host/adopt_advice_test.go| index fc5faebd3..9c4681c68 100644 |
| --- a/internal/host/adopt_advice_test.go |
| +++ b/internal/host/adopt_advice_test.go |
| @@ -103,11 +103,21 @@ func TestParseNftSkuids(t *testing.T) { |
| tcp dport 22 skuid 1000 accept |
| } |
| }` |
| + // The shape the infra repo's scripts/harden-host.sh actually writes: |
| + // inline, numeric, `meta skuid <n> accept`, one rule per allowed uid, |
| + // with a bare drop closing the gate. No named sets today. |
| + hardenHost := `ip daddr 169.254.169.254 meta skuid 0 accept |
| +ip daddr 169.254.169.254 meta skuid 998 accept |
| +ip daddr 169.254.169.254 drop` |
| cases := []struct { |
| name string |
| out string |
| want map[int]bool |
| }{ |
| + {"harden-host.sh's own rule shape", hardenHost, map[int]bool{0: true, 998: true}}, |
| + {"a DROP rule is not an allowlist entry", "ip daddr 169.254.169.254 meta skuid 998 drop\n", map[int]bool{}}, |
| + {"a REJECT rule is not one either", "ip daddr 169.254.169.254 meta skuid 998 reject\n", map[int]bool{}}, |
| + {"a rule with no verdict allows nothing", "ip daddr 169.254.169.254 meta skuid 998 counter\n", map[int]bool{}}, |
| {"two IMDS rules, and an unrelated skuid rule ignored", realistic, map[int]bool{0: true, 998: true}}, |
| {"set form", "ip daddr 169.254.169.254 skuid { 998, 999 } accept\n", map[int]bool{998: true, 999: true}}, |
| {"no IMDS rule at all", "tcp dport 22 skuid 1000 accept\n", map[int]bool{}}, |
| @@ -222,3 +232,25 @@ func TestAdviseHostRoleGateMissingIsQuietWithNothingToSay(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// A readable ruleset with NO IMDS gate in it is a third fact, with a third |
| +// fix: the box is not gated at all, so the answer is not "add a uid" but |
| +// "harden this box first" (tito has no gate today). It must not be folded |
| +// into either of the other two lines, and it must not go quiet — a host_role |
| +// app on an ungated box is F1's exposure with nothing in front of it. |
| +func TestAdviseHostRoleGateMissingNamesAnUngatedBox(t *testing.T) { |
| + granted := registry.Route{Host: "keymail.dev", Kind: "instance", App: "keymail", Backing: registry.BackingUnit, Status: "ready", Caps: "host-role"} |
| + logged := captureLog(t) |
| + adviseHostRoleGateMissing([]registry.Route{granted}, |
| + func(string) (int, bool) { return 998, true }, |
| + func() (map[int]bool, bool) { return map[int]bool{}, true }) |
| + out := logged() |
| + for _, w := range []string{"no IMDS uid gate found", "scripts/harden-host.sh", "uid 998"} { |
| + if !strings.Contains(out, w) { |
| + t.Errorf("log = %q, want it to contain %q", out, w) |
| + } |
| + } |
| + if strings.Contains(out, "could not be read") { |
| + t.Errorf("log = %q, want the ungated-box claim, not the unreadable-gate one", out) |
| + } |
| +} |
diff --git a/internal/host/caps_pass.go b/internal/host/caps_pass.go| index 3ce63f461..e932ce3eb 100644 |
| --- a/internal/host/caps_pass.go |
| +++ b/internal/host/caps_pass.go |
| @@ -25,6 +25,7 @@ import ( |
| "log" |
| "path/filepath" |
| "sort" |
| + "strings" |
| "github.com/carlosframework/platform/internal/caps" |
| "github.com/carlosframework/platform/internal/registry" |
| @@ -77,6 +78,17 @@ func capabilityPass(reg *registry.Registry, optDir string, o caps.Ops) error { |
| // here. DeferRestart stays FALSE (caps.Plan's contract): restarting |
| // the live binary to pick up a new unit shape is the entire point of |
| // this pass, and nothing here follows up with a Restart. |
| + // |
| + // Both returns are discarded ON PURPOSE, and that is correct FOR THIS |
| + // CALLER only. Result.Changed together with an error means bytes are |
| + // still moved and Converge will not move them back — which a |
| + // version-change caller must answer with caps.Restore, because there |
| + // the moved bytes are the NEW version's shape under the OLD binary. |
| + // Here there is no version change and no old binary: the bytes moved |
| + // are this route's own live shape, which is the steady state this |
| + // pass is converging TOWARD, and the next pass simply finds whatever |
| + // diff remains and retries it. Adding a Restore here would undo a |
| + // correct repair every time a post-reload verb failed. |
| _, _ = caps.Converge(o, p) |
| } |
| @@ -138,10 +150,12 @@ func buildPlans(routes []registry.Route, load func(rt registry.Route) (*release. |
| var advice []string |
| for _, rt := range routes { |
| unitRoute := release.CapabilityRoute(rt) |
| - // The exec-backed candidate set is warnBackingMismatch's, exactly: |
| - // an instance route of a known app that is not a platform alias and |
| - // is not unit-backed. It materializes nothing (release.CapabilityRoute |
| - // excludes it) and is told so once per pass. |
| + // The exec-backed candidate set is warnBackingMismatch's plus one |
| + // condition: an instance route that is not a platform alias and is |
| + // not unit-backed, AND that names an app — without an app there is no |
| + // release, so nothing could declare a capability to advise about. It |
| + // materializes nothing (release.CapabilityRoute excludes it) and is |
| + // told so once per pass. |
| execCandidate := !unitRoute && rt.Kind == "instance" && rt.App != "" && !rt.Platform && rt.Backing != registry.BackingUnit |
| if !unitRoute && !execCandidate { |
| continue |
| @@ -149,7 +163,16 @@ func buildPlans(routes []registry.Route, load func(rt registry.Route) (*release. |
| m, err := load(rt) |
| if err != nil { |
| - advice = append(advice, fmt.Sprintf("adopt: %s: the staged manifest of its live version could not be read (%v) — its capability drop-ins are left exactly as they are until it can be", rt.Host, err)) |
| + // Two different consequences, so two different sentences: a unit |
| + // route HAS drop-ins that are being left alone, an exec-backed |
| + // route has none and never did — telling its operator that |
| + // something is being left as it is would describe a file that |
| + // does not exist. |
| + if unitRoute { |
| + advice = append(advice, fmt.Sprintf("adopt: %s: the staged manifest of its live version could not be read (%v) — its capability drop-ins are left exactly as they are until it can be", rt.Host, err)) |
| + } else { |
| + advice = append(advice, fmt.Sprintf("adopt: %s: the staged manifest of its live version could not be read (%v) — this pass cannot tell whether it declares capabilities; it is exec-backed, so nothing is materialized for it either way", rt.Host, err)) |
| + } |
| continue |
| } |
| @@ -224,7 +247,7 @@ func dataAdvice(rt registry.Route, s *caps.Set) []string { |
| } |
| var out []string |
| if s.Data.Replicate == caps.DataReplicatePlatform && len(s.Data.DBs) > 1 { |
| - for _, extra := range sortedDBs(s.Data.DBs)[1:] { |
| + for _, extra := range s.Data.DBs[1:] { |
| out = append(out, fmt.Sprintf("adopt: %s declares data.dbs %q, but a route replicates exactly one database (DBPath is single-valued) — %q is NOT replicated by the box-wide litestream unit", |
| rt.Host, extra, extra)) |
| } |
| @@ -241,16 +264,33 @@ func dataAdvice(rt registry.Route, s *caps.Set) []string { |
| // current code documents as "silently replicated by nobody" (spec §6). |
| // |
| // Only `data.replicate: "platform"` with a non-empty dbs list stamps, only |
| -// the first name in sorted order, and only when the value would actually |
| -// change: this runs hourly, and a write per pass would be a registry |
| -// heartbeat nobody asked for. |
| +// dbs[0] AS DECLARED, and only when the value would actually change: this |
| +// runs hourly, and a write per pass would be a registry heartbeat nobody |
| +// asked for. |
| +// |
| +// dbs[0] as declared, NOT sorted (orchestrator ruling, 2026-08-03, |
| +// overruling the plan): an author writing ["console.db", "audit.db"] means |
| +// console.db is the one that matters, and lexicographic order would have |
| +// replicated audit.db instead — the platform silently picking a different |
| +// database than the app named. Declaration order is the app's own statement |
| +// of which database is primary; nothing else in the manifest carries it. |
| +// |
| +// The overwrite rule is the sharp one, and it is deliberately conservative |
| +// (2026-08-03 review, I3). An existing NON-EMPTY db_path this pass does not |
| +// recognise — anything not already under this box's state root for this host |
| +// — is an operator's `-db`, pointing at where the data lives RIGHT NOW. On a |
| +// pre-migration route (design §5) that is /data/<host>.db, the file the |
| +// box-wide replicator is streaming today; overwriting it with the |
| +// post-migration path would point litestream at a file that does not exist |
| +// yet and silently stop replicating a live database. So it is advised and |
| +// left alone, and the stamp follows the operator's data copy. |
| // |
| // It is deliberately NOT gated on the route's converge succeeding. The state |
| // directory is the unchanged template's own StateDirectory=carlos-app/%i, so |
| // the path is right whether or not a drop-in was written this pass, and a |
| // route refused for an unrelated reason (a port another route already claims) |
| -// should still have its database replicated. FleetDBs stats every path and |
| -// skips what is not there, so a stamp that precedes the file is harmless. |
| +// must still have its database replicated — a capability refusal is not a |
| +// reason to stop backing up somebody's data. |
| // |
| // The state root comes from the same Ops the converge used, so a test's temp |
| // dir is stamped rather than a production path a test never wrote to. |
| @@ -271,23 +311,35 @@ func stampDBPaths(reg *registry.Registry, routes []registry.Route, plans []caps. |
| if d.Replicate != caps.DataReplicatePlatform || len(d.DBs) == 0 { |
| continue |
| } |
| - want := filepath.Join(root, p.Host, sortedDBs(d.DBs)[0]) |
| - if cur[p.Host] == want { |
| + want := filepath.Join(root, p.Host, d.DBs[0]) |
| + from := cur[p.Host] |
| + if from == want { |
| continue |
| } |
| - if err := reg.SetDBPath(p.Host, want); err != nil { |
| + if from != "" && !ownStateDirPath(from, root, p.Host) { |
| + log.Printf("adopt: %s carries db_path %s, which is not under this box's state directory for it; the manifest expects %s — leaving the operator's value alone, because repointing the replicator at a file the §5 data copy has not created yet would silently stop replicating a live database. The stamp follows the copy", |
| + p.Host, from, want) |
| + continue |
| + } |
| + if err := reg.SetDBPath(p.Host, from, want); err != nil { |
| log.Printf("adopt: %s: recording db_path %s: %v", p.Host, want, err) |
| continue |
| } |
| - log.Printf("adopt: %s: db_path recorded as %s from the app's data.dbs — the box-wide litestream unit replicates it from the next `carlos ops litestream` run", p.Host, want) |
| + if from == "" { |
| + log.Printf("adopt: %s: db_path recorded as %s from the app's data.dbs — the box-wide litestream unit replicates it from the next `carlos ops litestream` run", p.Host, want) |
| + } else { |
| + log.Printf("adopt: %s: db_path %s -> %s, following the app's data.dbs — the box-wide litestream unit picks the new file up on the next `carlos ops litestream` run", p.Host, from, want) |
| + } |
| } |
| } |
| -// sortedDBs is data.dbs in a deterministic order, on a COPY: the slice |
| -// belongs to a decoded manifest that other readers share, and an in-place |
| -// sort here would reorder what they see. |
| -func sortedDBs(dbs []string) []string { |
| - out := append([]string(nil), dbs...) |
| - sort.Strings(out) |
| - return out |
| +// ownStateDirPath reports whether path is a file this box's own state |
| +// directory for host owns — i.e. a value some pass of this function wrote, |
| +// or would write. Compared as a cleaned path prefix so a trailing slash or a |
| +// "." segment cannot dodge it, and never as a substring: /var/lib/carlos-app/ |
| +// <host>-old/x must not read as this host's. |
| +func ownStateDirPath(path, root, host string) bool { |
| + dir := filepath.Clean(filepath.Join(root, host)) |
| + rel, err := filepath.Rel(dir, filepath.Clean(path)) |
| + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) |
| } |
diff --git a/internal/host/caps_pass_test.go b/internal/host/caps_pass_test.go| index 51cfe8947..afa428bba 100644 |
| --- a/internal/host/caps_pass_test.go |
| +++ b/internal/host/caps_pass_test.go |
| @@ -25,11 +25,16 @@ type passRec struct { |
| calls [][]string |
| users []string |
| logs []string |
| + // query is what systemd would answer for is-enabled/is-active. It |
| + // defaults to "yes to everything" — a converged, running box, where the |
| + // pass has no drift to repair — and a test swaps it to make systemd |
| + // disagree with the declaration. |
| + query func(args ...string) bool |
| } |
| func passOps(t *testing.T) (caps.Ops, *passRec) { |
| t.Helper() |
| - rec := &passRec{} |
| + rec := &passRec{query: func(...string) bool { return true }} |
| return caps.Ops{ |
| EtcDir: t.TempDir(), |
| StateRoot: t.TempDir(), |
| @@ -38,7 +43,7 @@ func passOps(t *testing.T) (caps.Ops, *passRec) { |
| rec.calls = append(rec.calls, args) |
| return nil |
| }, |
| - Query: func(args ...string) bool { return true }, |
| + Query: func(args ...string) bool { return rec.query(args...) }, |
| EnsureUser: func(name string) error { rec.users = append(rec.users, name); return nil }, |
| Logf: func(f string, a ...any) { rec.logs = append(rec.logs, strings.TrimSpace(fmt.Sprintf(f, a...))) }, |
| }, rec |
| @@ -289,7 +294,11 @@ func TestCapabilityPassStampsDBPathFromDataDBs(t *testing.T) { |
| o, _ := passOps(t) |
| stageVersion(t, optDir, "console", "aaaaaaa", &caps.Set{ |
| V: 1, Daemon: true, |
| - Data: &caps.Data{Replicate: caps.DataReplicatePlatform, DBs: []string{"zebra.db", "console.db"}}, |
| + // Declared order, NOT sorted: an author writing console.db first |
| + // means console.db is the one that matters, and a lexicographic sort |
| + // would have replicated audit.db instead — the platform silently |
| + // picking a different database than the app named. |
| + Data: &caps.Data{Replicate: caps.DataReplicatePlatform, DBs: []string{"console.db", "audit.db"}}, |
| }) |
| if err := release.PointLive(optDir, "console.example.com", release.OpsAccount, "aaaaaaa", "console"); err != nil { |
| t.Fatal(err) |
| @@ -304,7 +313,7 @@ func TestCapabilityPassStampsDBPathFromDataDBs(t *testing.T) { |
| } |
| want := filepath.Join(o.StateRoot, "console.example.com", "console.db") |
| if got.DBPath != want { |
| - t.Errorf("DBPath = %q, want %q (dbs[0] sorted, under the state dir)", got.DBPath, want) |
| + t.Errorf("DBPath = %q, want %q (dbs[0] AS DECLARED, under the state dir)", got.DBPath, want) |
| } |
| } |
| @@ -568,17 +577,170 @@ func TestBuildPlansAdvisesUnreplicatedExtraDBs(t *testing.T) { |
| V: 1, Data: &caps.Data{Replicate: caps.DataReplicatePlatform, DBs: []string{"console.db", "sessions.db", "audit.db"}}, |
| })}, nil)) |
| joined := strings.Join(advice, "\n") |
| - // Sorted, so audit.db is the one that gets the DBPath stamp and the other |
| - // two are the ones an operator has to hear about by name. |
| - for _, w := range []string{`"console.db" is NOT replicated`, `"sessions.db" is NOT replicated`} { |
| + // dbs[0] AS DECLARED is the replicated one, so the two the operator has |
| + // to hear about by name are the ones after it — in declaration order, |
| + // not lexicographic order. |
| + for _, w := range []string{`"sessions.db" is NOT replicated`, `"audit.db" is NOT replicated`} { |
| if !strings.Contains(joined, w) { |
| t.Errorf("advice =\n%s\nwant a line containing %q", joined, w) |
| } |
| } |
| - if strings.Contains(joined, `"audit.db" is NOT replicated`) { |
| - t.Errorf("advice =\n%s\nwant the FIRST db (audit.db, sorted) treated as replicated", joined) |
| + if strings.Contains(joined, `"console.db" is NOT replicated`) { |
| + t.Errorf("advice =\n%s\nwant the FIRST db AS DECLARED (console.db) treated as replicated", joined) |
| } |
| if len(advice) != 2 { |
| t.Errorf("advice = %v, want exactly one line per unreplicated extra", advice) |
| } |
| } |
| + |
| +// A pass that writes no bytes still repairs UNIT STATE — and until the review |
| +// that found this, it did so in total silence. The scenario is an operator |
| +// who stopped carlos-app@keymail.dev by hand to debug it: an hour later the |
| +// pass starts it again, correctly, and the log said nothing at all about the |
| +// most surprising thing this pass does. The start must happen AND be |
| +// announced with the verb and the reason. |
| +func TestCapabilityPassAnnouncesARepairOnABytesUnchangedPass(t *testing.T) { |
| + optDir := t.TempDir() |
| + o, rec := passOps(t) |
| + stageVersion(t, optDir, "keymail", "aaaaaaa", &caps.Set{V: 1, Daemon: true, Children: true}) |
| + if err := release.PointLive(optDir, "keymail.dev", release.OpsAccount, "aaaaaaa", "keymail"); err != nil { |
| + t.Fatal(err) |
| + } |
| + reg := openReg(t, unitRoute("keymail.dev", "keymail")) |
| + if err := capabilityPass(reg, optDir, o); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // Second pass: byte-identical drop-ins, but systemd says the service is |
| + // not running. |
| + rec.calls, rec.logs = nil, nil |
| + rec.query = func(args ...string) bool { |
| + return !(len(args) == 2 && args[0] == "is-active" && args[1] == caps.ServiceUnit("keymail.dev")) |
| + } |
| + if err := capabilityPass(reg, optDir, o); err != nil { |
| + t.Fatal(err) |
| + } |
| + if !recordedVerb(rec, "start", caps.ServiceUnit("keymail.dev")) { |
| + t.Errorf("systemctl calls = %v, want the stopped daemon started", rec.calls) |
| + } |
| + joined := strings.Join(rec.logs, "\n") |
| + if !strings.Contains(joined, "was not running") || !strings.Contains(joined, "started it") { |
| + t.Errorf("logs = %v, want a line naming the verb and why it fired", rec.logs) |
| + } |
| +} |
| + |
| +// An operator's -db value is a path to where the data lives RIGHT NOW. On a |
| +// pre-migration route that is /data/<host>.db, the file the box-wide |
| +// replicator is streaming today — repointing it at the post-migration path |
| +// before the §5 copy has happened would silently stop replicating a live |
| +// database. So it is advised and left alone. |
| +func TestCapabilityPassLeavesAnOperatorsDBPathAloneAndSaysWhy(t *testing.T) { |
| + optDir := t.TempDir() |
| + o, _ := passOps(t) |
| + stageVersion(t, optDir, "console", "aaaaaaa", &caps.Set{ |
| + V: 1, Data: &caps.Data{Replicate: caps.DataReplicatePlatform, DBs: []string{"console.db"}}, |
| + }) |
| + if err := release.PointLive(optDir, "console.example.com", release.OpsAccount, "aaaaaaa", "console"); err != nil { |
| + t.Fatal(err) |
| + } |
| + rt := unitRoute("console.example.com", "console") |
| + rt.DBPath = "/data/console.example.com.db" |
| + reg := openReg(t, rt) |
| + |
| + logged := captureLog(t) |
| + if err := capabilityPass(reg, optDir, o); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, err := reg.Get("console.example.com") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if got.DBPath != "/data/console.example.com.db" { |
| + t.Errorf("DBPath = %q, want the operator's value untouched", got.DBPath) |
| + } |
| + out := logged() |
| + for _, w := range []string{"/data/console.example.com.db", filepath.Join(o.StateRoot, "console.example.com", "console.db"), "stamp follows the copy"} { |
| + if !strings.Contains(out, w) { |
| + t.Errorf("log = %q, want it to name %q", out, w) |
| + } |
| + } |
| +} |
| + |
| +// A value this pass itself wrote — under the box's own state dir for this |
| +// host — IS corrected when the manifest's first database changes, and the log |
| +// names the old value, because "db_path changed" with no `from` is |
| +// unreviewable in an hourly log. |
| +func TestCapabilityPassCorrectsItsOwnStaleDBPathNamingTheOldValue(t *testing.T) { |
| + optDir := t.TempDir() |
| + o, _ := passOps(t) |
| + stageVersion(t, optDir, "console", "aaaaaaa", &caps.Set{ |
| + V: 1, Data: &caps.Data{Replicate: caps.DataReplicatePlatform, DBs: []string{"console.db"}}, |
| + }) |
| + if err := release.PointLive(optDir, "console.example.com", release.OpsAccount, "aaaaaaa", "console"); err != nil { |
| + t.Fatal(err) |
| + } |
| + stale := filepath.Join(o.StateRoot, "console.example.com", "old.db") |
| + rt := unitRoute("console.example.com", "console") |
| + rt.DBPath = stale |
| + reg := openReg(t, rt) |
| + |
| + logged := captureLog(t) |
| + if err := capabilityPass(reg, optDir, o); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, err := reg.Get("console.example.com") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want := filepath.Join(o.StateRoot, "console.example.com", "console.db") |
| + if got.DBPath != want { |
| + t.Errorf("DBPath = %q, want %q — a value under this box's own state dir is this pass's to correct", got.DBPath, want) |
| + } |
| + if out := logged(); !strings.Contains(out, stale) || !strings.Contains(out, want) { |
| + t.Errorf("log = %q, want it to name both the old and the new value", out) |
| + } |
| +} |
| + |
| +// A route whose converge was REFUSED still gets its database replicated. The |
| +// refusal here is the box-wide port claim (two routes granted :25, the |
| +// lexicographically later one refused), and it says nothing about the app's |
| +// data: gating the stamp on converge success would stop backing up a live |
| +// database because of an unrelated capability conflict. |
| +func TestCapabilityPassStampsDBPathEvenWhenConvergeIsRefused(t *testing.T) { |
| + optDir := t.TempDir() |
| + o, _ := passOps(t) |
| + set := &caps.Set{ |
| + V: 1, Daemon: true, |
| + Ports: []caps.Port{{Port: 25}}, |
| + Data: &caps.Data{Replicate: caps.DataReplicatePlatform, DBs: []string{"console.db"}}, |
| + } |
| + stageVersion(t, optDir, "console", "aaaaaaa", set) |
| + for _, h := range []string{"a.example.com", "b.example.com"} { |
| + if err := release.PointLive(optDir, h, release.OpsAccount, "aaaaaaa", "console"); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + routes := make([]registry.Route, 0, 2) |
| + for _, h := range []string{"a.example.com", "b.example.com"} { |
| + rt := unitRoute(h, "console") |
| + rt.Caps = "port:25" |
| + routes = append(routes, rt) |
| + } |
| + reg := openReg(t, routes...) |
| + |
| + if err := capabilityPass(reg, optDir, o); err != nil { |
| + t.Fatal(err) |
| + } |
| + // b sorts later, so CheckClaims refuses it and nothing is written for it. |
| + if _, sock := readDropIns(t, o, "b.example.com"); sock != "" { |
| + t.Fatalf("b.example.com socket drop-in = %q, want nothing written for a refused route", sock) |
| + } |
| + got, err := reg.Get("b.example.com") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want := filepath.Join(o.StateRoot, "b.example.com", "console.db") |
| + if got.DBPath != want { |
| + t.Errorf("DBPath = %q, want %q — a capability refusal is not a reason to stop replicating data", got.DBPath, want) |
| + } |
| +} |
diff --git a/internal/registry/registry.go b/internal/registry/registry.go| index 9f7c29f64..a14c46ad1 100644 |
| --- a/internal/registry/registry.go |
| +++ b/internal/registry/registry.go |
| @@ -593,8 +593,58 @@ func capsOrNone(caps string) string { |
| // Errors on an unmatched host, like SetChannel and SetCaps: the caller is a |
| // pass that has just read the row, so "the row is gone" is a race worth |
| // reporting rather than swallowing. |
| -func (r *Registry) SetDBPath(host, path string) error { |
| - res, err := r.sql.Exec(`UPDATE routes SET db_path = ? WHERE host = ?`, path, host) |
| +// |
| +// Compare-and-swap on the column's prior value, the same shape and the same |
| +// reason as SetCaps: the caller decided to write `to` BECAUSE it read `from` |
| +// — the whole stamp rule is "only when it differs, and only over a value this |
| +// pass recognises". An hourly root pass racing an operator's `carlos add -db` |
| +// (or a second box's pass against a shared registry) would otherwise overwrite |
| +// a path nobody has looked at, and the column decides which file the box-wide |
| +// replicator streams. A lost race is a refusal naming the current value, and |
| +// the next pass re-decides against reality. |
| +func (r *Registry) SetDBPath(host, from, to string) error { |
| + res, err := r.sql.Exec(`UPDATE routes SET db_path = ? WHERE host = ? AND db_path = ?`, to, host, from) |
| + if err != nil { |
| + return err |
| + } |
| + n, err := res.RowsAffected() |
| + if err != nil { |
| + return err |
| + } |
| + if n == 0 { |
| + var cur string |
| + serr := r.sql.QueryRow(`SELECT db_path FROM routes WHERE host = ?`, host).Scan(&cur) |
| + if serr == sql.ErrNoRows { |
| + return fmt.Errorf("no route for host %q", host) |
| + } |
| + if serr != nil { |
| + return serr |
| + } |
| + return fmt.Errorf("db_path for %q changed underneath this pass (now %q) — leaving it alone; the next pass decides against the new value", host, cur) |
| + } |
| + return nil |
| +} |
| + |
| +// SetBacking stamps who owns a route's process: "" (an exec child of the |
| +// agent) or BackingUnit (a carlos-app@<host> systemd unit). It is the door |
| +// the app-capabilities migration (design §5b, §5c) needs and did not have: |
| +// backing could be chosen at `carlos add` and never changed afterwards, so |
| +// the reversible cutover — stop the hand-provisioned unit, copy the data, |
| +// stamp the row, let the adopt pass converge it — had no way to stamp, and |
| +// the reverse move no way to undo. |
| +// |
| +// Narrow UPDATE with an error on an unmatched host, like SetChannel and |
| +// SetCaps: a human types the hostname into this one directly, and a silent |
| +// no-op on a typo would be worse than a refusal. It validates nothing — the |
| +// value rail and the row-ownership rails live at the door in cli.go, exactly |
| +// as they do for the caps column. |
| +// |
| +// Deliberately NOT a compare-and-swap. Backing is not a set an operator |
| +// merges into (SetCaps's case) and not a value an hourly pass rewrites |
| +// (SetDBPath's): it is one operator, at one step of a documented migration, |
| +// naming the value they want. |
| +func (r *Registry) SetBacking(host, backing string) error { |
| + res, err := r.sql.Exec(`UPDATE routes SET backing = ? WHERE host = ?`, backing, host) |
| if err != nil { |
| return err |
| } |
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go| index c331c00e9..cf6d3012b 100644 |
| --- a/internal/registry/registry_test.go |
| +++ b/internal/registry/registry_test.go |
| @@ -908,7 +908,7 @@ func TestSetDBPathIsNarrowAndErrorsOnAnUnknownHost(t *testing.T) { |
| if err := reg.SetCaps("console.example.com", "", "host-role"); err != nil { |
| t.Fatal(err) |
| } |
| - if err := reg.SetDBPath("console.example.com", "/var/lib/carlos-app/console.example.com/console.db"); err != nil { |
| + if err := reg.SetDBPath("console.example.com", "", "/var/lib/carlos-app/console.example.com/console.db"); err != nil { |
| t.Fatal(err) |
| } |
| got, err := reg.Get("console.example.com") |
| @@ -922,9 +922,58 @@ func TestSetDBPathIsNarrowAndErrorsOnAnUnknownHost(t *testing.T) { |
| if *got != want { |
| t.Fatalf("SetDBPath was not narrow:\n got %+v\n want %+v", *got, want) |
| } |
| - if err := reg.SetDBPath("nope.example.com", "/x"); err == nil { |
| + if err := reg.SetDBPath("nope.example.com", "", "/x"); err == nil { |
| t.Fatal("SetDBPath on an unknown host must error") |
| } |
| + // Compare-and-swap, like SetCaps: the hourly pass decided to write |
| + // BECAUSE of the value it read, so a value that moved underneath it is a |
| + // refusal naming the current one — never a blind overwrite of a path |
| + // somebody else just set, which decides which file gets replicated. |
| + if err := reg.SetDBPath("console.example.com", "/data/stale.db", "/var/lib/carlos-app/console.example.com/other.db"); err == nil { |
| + t.Fatal("SetDBPath with a stale prior value must refuse, not overwrite") |
| + } else if !strings.Contains(err.Error(), "changed underneath") || !strings.Contains(err.Error(), "console.db") { |
| + t.Fatalf("a lost race should name the condition and the current value, got: %v", err) |
| + } |
| +} |
| + |
| +// SetBacking is the migration door (app-capabilities design §5b/§5c): it |
| +// moves a route between an exec child of the agent and a carlos-app@<host> |
| +// unit, in both directions, narrowly — every other column of the row is |
| +// untouched, so a stamp mid-migration cannot lose a grant or a version. |
| +func TestSetBackingIsNarrowAndReversible(t *testing.T) { |
| + reg, err := Open(filepath.Join(t.TempDir(), "registry.db")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + base := Route{Host: "console.example.com", Kind: "instance", App: "console", Channel: "canary/console", Status: "ready", DBPath: "/data/console.db"} |
| + if err := reg.Upsert(base); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.SetBacking("console.example.com", BackingUnit); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, err := reg.Get("console.example.com") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want := base |
| + want.Backing = BackingUnit |
| + want.CreatedAt = got.CreatedAt |
| + if *got != want { |
| + t.Fatalf("SetBacking was not narrow:\n got %+v\n want %+v", *got, want) |
| + } |
| + // Reversible: the migration's rollback step is the same verb, back. |
| + if err := reg.SetBacking("console.example.com", ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got, err := reg.Get("console.example.com"); err != nil { |
| + t.Fatal(err) |
| + } else if got.Backing != "" { |
| + t.Fatalf("backing = %q, want it back to the exec child", got.Backing) |
| + } |
| + if err := reg.SetBacking("nope.example.com", BackingUnit); err == nil { |
| + t.Fatal("SetBacking on an unknown host must error") |
| + } |
| } |
| // The named finding, as a test: `carlos add` re-runs and the domains |