fix(#3987): join the agent-status key with a dot so a hive's write grant can be scoped
A KV entry publishes to $KV.<bucket>.<key> and NATS wildcards match whole
.-delimited tokens, so a /-joined {hive}/{agent} key is a single token: the
only expressible write grants are one exact subject per agent (needs a roster
in the auth responder, which Policy::hive_name argues against) or a bucket-wide
wildcard that lets any hive overwrite any other hive's agents.
Joining with a dot puts the hive in its own token, so hive_subjects can grant
$KV.agent-status.<hive>.* — every agent of one hive and nothing else, the same
containment hive-status already has.
The grant lands with the bucket-open pair (STREAM.INFO + STREAM.CREATE):
open_or_create resolves the bucket before it writes, so alone the publish
subject is unreachable and the sweep fails one step later instead.
Verified the client accepts a dotted key rather than assuming it: async-nats
0.50.0 VALID_KEY_RE is \A[-/_=.a-zA-Z0-9]+\z and is_valid_key rejects only
empty / leading / trailing dot; the subject is prefix + key verbatim.
This commit is contained in:
parent
c9d8628794
commit
3b038425f2
2 changed files with 109 additions and 34 deletions
|
|
@ -271,6 +271,31 @@ impl Policy {
|
|||
swarm_queue_client::wanted::BUCKET
|
||||
),
|
||||
]);
|
||||
// Publishing this hive's agents into the per-agent status bucket, and
|
||||
// **only its own agents**.
|
||||
//
|
||||
// `.*` matches exactly one token, so this is every agent of one hive
|
||||
// and nothing deeper — `.>` would also admit `<hive>.<agent>.<more>`,
|
||||
// which `agent_status::split_key` rejects on read anyway; the grant has
|
||||
// no reason to be wider than the format.
|
||||
//
|
||||
// The scoping is only expressible because the key is `.`-joined: a KV
|
||||
// entry publishes to `$KV.<bucket>.<key>`, and a NATS wildcard matches
|
||||
// whole tokens, so a key joined by anything else is one token and
|
||||
// leaves only "one exact subject per agent" (needs a roster in here,
|
||||
// which `hive_name`'s doc argues against) or a bucket-wide wildcard
|
||||
// (any hive overwrites any other hive's agents).
|
||||
//
|
||||
// `STREAM.INFO` + `CREATE` come along for the same reason they do
|
||||
// above: `open_or_create` resolves the bucket before it writes, and
|
||||
// either end may be first on a fresh swarm. Without them the publish
|
||||
// grant is unreachable — the client times out at bucket open.
|
||||
let agent_status = Self::agent_status_stream();
|
||||
subjects.extend([
|
||||
format!("$JS.API.STREAM.INFO.{agent_status}"),
|
||||
format!("$JS.API.STREAM.CREATE.{agent_status}"),
|
||||
format!("$KV.{}.{hive}.*", swarm_queue_client::agent_status::BUCKET),
|
||||
]);
|
||||
subjects.extend(
|
||||
self.extra_hive_subjects
|
||||
.iter()
|
||||
|
|
@ -338,12 +363,10 @@ impl Policy {
|
|||
// ephemeral consumer whose subject carries no name.
|
||||
//
|
||||
// Deliberately **no `$KV.<bucket>.…` subject**: that is the write
|
||||
// side, and writing is what picks a key layout. Who publishes agent
|
||||
// status, and under which key, is still open — so granting a write
|
||||
// subject here would answer a question this change has no business
|
||||
// answering. Reading needs none of it: a KV read is a `DIRECT.GET`,
|
||||
// and the `.>` form is bucket-wide rather than per-key, so nothing
|
||||
// below encodes a layout.
|
||||
// side, and the controller only reads this bucket. The hive gets a
|
||||
// write grant scoped to its own agents (see `hive_subjects`); a
|
||||
// reader holding one could forge any agent's status on any hive.
|
||||
// Reading needs none of it — a KV read is a `DIRECT.GET`.
|
||||
let agent_status = Self::agent_status_stream();
|
||||
subjects.extend([
|
||||
format!("$JS.API.STREAM.INFO.{agent_status}"),
|
||||
|
|
@ -391,26 +414,54 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn no_grant_encodes_an_agent_status_key_layout() {
|
||||
// Who writes agent status, and under which key, is undecided. A `$KV.`
|
||||
// subject is the write side and would answer that by implication, so
|
||||
// NOBODY gets one for this bucket until the layout is settled.
|
||||
fn the_reader_gets_no_write_subject_for_the_agent_status_bucket() {
|
||||
// The controller reads this bucket and never writes it. A `$KV.`
|
||||
// subject here would let a reader forge any agent's status on any
|
||||
// hive — the containment `hive_subjects` establishes is only worth
|
||||
// anything if the other role cannot bypass it.
|
||||
let bucket = swarm_queue_client::agent_status::BUCKET;
|
||||
let prefix = format!("$KV.{bucket}.");
|
||||
let p = policy();
|
||||
for (who, subjects) in [
|
||||
("reader", p.reader_subjects()),
|
||||
("hive", p.hive_subjects("alpha")),
|
||||
let subjects = policy().reader_subjects();
|
||||
assert!(
|
||||
!subjects.iter().any(|s| s.starts_with(&prefix)),
|
||||
"the reader was granted a {prefix}* subject"
|
||||
);
|
||||
// Control: the prefix test does fire on a bucket the reader
|
||||
// legitimately writes, so the pass above is about agent-status rather
|
||||
// than about a matcher that never matches anything.
|
||||
let wanted = format!("$KV.{}.", swarm_queue_client::wanted::BUCKET);
|
||||
assert!(subjects.iter().any(|s| s.starts_with(&wanted)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_hive_may_write_only_its_own_agents_status_keys() {
|
||||
let bucket = swarm_queue_client::agent_status::BUCKET;
|
||||
let subjects = policy().hive_subjects("alpha");
|
||||
let s = Policy::agent_status_stream();
|
||||
for want in [
|
||||
// Without these the write subject is unreachable: `open_or_create`
|
||||
// resolves the bucket first and times out before it publishes.
|
||||
format!("$JS.API.STREAM.INFO.{s}"),
|
||||
format!("$JS.API.STREAM.CREATE.{s}"),
|
||||
format!("$KV.{bucket}.alpha.*"),
|
||||
] {
|
||||
assert!(subjects.contains(&want), "hive alpha is missing {want}");
|
||||
}
|
||||
// The scoping is the point, so pin what must NOT be there: a
|
||||
// bucket-wide wildcard in any of its spellings, and another hive's
|
||||
// slice. `.*` matches one token; `.>` would match deeper than the key
|
||||
// format ever produces.
|
||||
for forbidden in [
|
||||
format!("$KV.{bucket}.>"),
|
||||
format!("$KV.{bucket}.*"),
|
||||
format!("$KV.{bucket}.alpha.>"),
|
||||
format!("$KV.{bucket}.beta.*"),
|
||||
] {
|
||||
assert!(
|
||||
!subjects.iter().any(|s| s.starts_with(&prefix)),
|
||||
"{who} was granted a {prefix}* subject, which picks a key layout"
|
||||
!subjects.contains(&forbidden),
|
||||
"hive alpha was granted {forbidden}, which reaches past its own agents"
|
||||
);
|
||||
}
|
||||
// Control: the prefix test does fire on a bucket that legitimately has
|
||||
// write grants, so a pass above is about agent-status, not the matcher.
|
||||
let wanted = format!("$KV.{}.", swarm_queue_client::wanted::BUCKET);
|
||||
assert!(p.reader_subjects().iter().any(|s| s.starts_with(&wanted)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
Loading…
Reference in a new issue