tasks: add ADR-050 implementation tasks — ownership store, resource_id_path, check signature, dispatch wiring
Four tasks forming a DAG for the dynamic resource ownership model (ADR-050): 1. core/ownership-store-trait (no deps) — OwnershipProvider (sync read) + OwnershipStore (async write) traits + InMemoryOwnershipStore + OwnershipError in alknet-core; fourth instance of the repo/adapter pattern (ADR-033) 2. call/registry/operation-spec-resource-id-path (no deps) — add resource_id_path: Option<String> to OperationSpec (JSON pointer into input for resource ID extraction) 3. call/registry/access-control-ownership-check (depends on 1) — update AccessControl::check signature to accept resource_id + OwnershipProvider; backward compatible (ownership=None falls back to static Identity.resources) 4. call/registry/dispatch-resource-id-extraction (depends on 2, 3) — wire dispatch path to extract resource_id from input via spec.resource_id_path and thread OwnershipProvider to check(); OperationContext gains ownership field Tasks 1 and 2 can run in parallel (different crates, no deps). Task 3 depends on 1. Task 4 depends on 2 and 3. Validated: no cycles, 90 tasks total.
This commit is contained in:
153
tasks/call/registry/access-control-ownership-check.md
Normal file
153
tasks/call/registry/access-control-ownership-check.md
Normal file
@@ -0,0 +1,153 @@
|
|||||||
|
---
|
||||||
|
id: call/registry/access-control-ownership-check
|
||||||
|
name: Update AccessControl::check to consult OwnershipProvider for dynamic resource ownership (ADR-050 §2)
|
||||||
|
status: pending
|
||||||
|
depends_on: [core/ownership-store-trait]
|
||||||
|
scope: moderate
|
||||||
|
risk: medium
|
||||||
|
impact: component
|
||||||
|
level: implementation
|
||||||
|
---
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Update `AccessControl::check` to accept `resource_id` and an optional
|
||||||
|
`OwnershipProvider` reference, consulting the provider for runtime-spawned
|
||||||
|
resource ownership checks. Per ADR-050 §2.
|
||||||
|
|
||||||
|
### The signature change
|
||||||
|
|
||||||
|
Current (in `crates/alknet-call/src/registry/spec.rs`):
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn check(&self, identity: Option<&Identity>) -> AccessResult
|
||||||
|
```
|
||||||
|
|
||||||
|
Updated:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn check(
|
||||||
|
&self,
|
||||||
|
identity: Option<&Identity>,
|
||||||
|
resource_id: Option<&str>,
|
||||||
|
ownership: Option<&dyn OwnershipProvider>,
|
||||||
|
) -> AccessResult
|
||||||
|
```
|
||||||
|
|
||||||
|
### Check logic
|
||||||
|
|
||||||
|
```
|
||||||
|
1. Scope check (unchanged): identity.scopes ⊇ required_scopes.
|
||||||
|
If identity is None and scopes are required, deny here.
|
||||||
|
(Same as current — no change to the scope-check path.)
|
||||||
|
|
||||||
|
2. Resource check (only if self.resource_type is Some):
|
||||||
|
a. resource_id Some + ownership Some:
|
||||||
|
→ identity must be Some (owns takes &Identity, not Option);
|
||||||
|
if identity is None, deny.
|
||||||
|
→ p.owns(identity, resource_type, resource_id, resource_action)
|
||||||
|
→ if false, Forbidden("not owner of resource")
|
||||||
|
b. resource_id None + ownership Some (the `list` case, ADR-050 §4a):
|
||||||
|
→ if identity is None, deny.
|
||||||
|
→ p.owns_any(identity, resource_type) [scope-gate]
|
||||||
|
→ if false, Forbidden("no owned resources of type")
|
||||||
|
c. ownership None → fall back to static
|
||||||
|
→ identity.resources[resource_type] ∋ resource_action
|
||||||
|
(backward compat for non-runtime resources — the current logic)
|
||||||
|
```
|
||||||
|
|
||||||
|
### Backward compatibility
|
||||||
|
|
||||||
|
When `ownership` is `None`, `check` falls back to the static
|
||||||
|
`Identity.resources` path — the existing resource-check logic (lines 83-105
|
||||||
|
of `spec.rs`) runs unchanged. This means operations with static resource
|
||||||
|
sets work without wiring an ownership provider. The ownership provider is
|
||||||
|
an additional check, not a replacement.
|
||||||
|
|
||||||
|
When `resource_id` is `None` and `ownership` is `None`, the existing logic
|
||||||
|
also runs unchanged (the `resource_action` without a specific resource ID
|
||||||
|
path, lines 97-105). This covers operations that have `resource_action` but
|
||||||
|
no `resource_type` — those stay on the static path.
|
||||||
|
|
||||||
|
### Call sites to update
|
||||||
|
|
||||||
|
There are **6 call sites** for `check()` that need the signature update:
|
||||||
|
|
||||||
|
1. `registry/registration.rs:142` — `acl.check(identity.as_ref())` in `invoke()`
|
||||||
|
2. `registry/registration.rs:194` — `acl.check(identity.as_ref())` in `invoke_streaming()`
|
||||||
|
3. `protocol/connection.rs:351` — `access_control.check(caller_identity.as_ref())` in `invoke_with_policy()`
|
||||||
|
4. `registry/discovery.rs:224` — `spec.access_control.check(calling_identity)` in `services/list`
|
||||||
|
5. `registry/discovery.rs:247` — `spec.access_control.check(calling_identity)` in `services/schema`
|
||||||
|
6. `registry/discovery.rs:269` — `reg.spec.access_control.check(calling_identity)` in `services/list-peers`
|
||||||
|
|
||||||
|
For sites 1-3 (dispatch path): pass `resource_id: None` and
|
||||||
|
`ownership: None` for now — the dispatch-path wiring (extracting
|
||||||
|
`resource_id` from input via `spec.resource_id_path` and threading the
|
||||||
|
ownership provider) is `call/registry/dispatch-resource-id-extraction`,
|
||||||
|
which depends on this task. This task changes the signature and the check
|
||||||
|
logic; the dispatch task wires the new parameters.
|
||||||
|
|
||||||
|
For sites 4-6 (service discovery): these are `services/list` /
|
||||||
|
`services/schema` / `services/list-peers` filtering — they check whether a
|
||||||
|
calling peer can see an operation at all. Pass `resource_id: None` and
|
||||||
|
`ownership: None` — service discovery doesn't do per-resource ownership
|
||||||
|
checks (it's scope-gating only). The `list` result-filter (ADR-050 §4a) is
|
||||||
|
a handler-level concern, not a service-discovery concern.
|
||||||
|
|
||||||
|
### What this task does NOT do
|
||||||
|
|
||||||
|
- Does NOT extract `resource_id` from the input or thread the ownership
|
||||||
|
provider into the dispatch path — that's
|
||||||
|
`call/registry/dispatch-resource-id-extraction`, which depends on this
|
||||||
|
task. This task changes the signature and the check logic; all call sites
|
||||||
|
pass `None, None` and behave exactly as before.
|
||||||
|
- Does NOT add `resource_id_path` to `OperationSpec` — that's
|
||||||
|
`call/registry/operation-spec-resource-id-path`, which is independent
|
||||||
|
(no dependency).
|
||||||
|
|
||||||
|
### Tests
|
||||||
|
|
||||||
|
The existing tests in `spec.rs` (lines 157-327) all call `check(identity)`
|
||||||
|
with one argument. They need to be updated to `check(identity, None, None)`
|
||||||
|
— the behavior should be identical (backward compat). Add new tests for
|
||||||
|
the ownership-provider path:
|
||||||
|
|
||||||
|
- `resource_id Some + ownership Some + provider says owns → Allowed`
|
||||||
|
- `resource_id Some + ownership Some + provider says not owns → Forbidden`
|
||||||
|
- `resource_id Some + ownership Some + identity None → Forbidden`
|
||||||
|
- `resource_id None + ownership Some + provider says owns_any → Allowed` (the `list` scope-gate)
|
||||||
|
- `resource_id None + ownership Some + provider says not owns_any → Forbidden`
|
||||||
|
- `ownership None + resource_type Some → static fallback (existing behavior)`
|
||||||
|
- A mock `OwnershipProvider` impl for testing
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
|
||||||
|
- [ ] `AccessControl::check` signature updated: `(identity, resource_id, ownership) -> AccessResult`
|
||||||
|
- [ ] Scope check path unchanged (required_scopes, required_scopes_any)
|
||||||
|
- [ ] Resource check path: when `ownership` is Some, consults the provider
|
||||||
|
- [ ] Resource check path: when `ownership` is None, falls back to static `Identity.resources` (backward compat)
|
||||||
|
- [ ] `resource_id None + ownership Some` → calls `owns_any` (the `list` scope-gate)
|
||||||
|
- [ ] `resource_id Some + ownership Some` → calls `owns`
|
||||||
|
- [ ] `identity None + ownership Some + resource_type Some` → Forbidden (owns needs &Identity)
|
||||||
|
- [ ] All 6 existing call sites updated to pass `(None, None)` — behavior unchanged
|
||||||
|
- [ ] All existing tests updated to `check(identity, None, None)` — behavior unchanged
|
||||||
|
- [ ] New tests with a mock `OwnershipProvider` for the dynamic path
|
||||||
|
- [ ] `cargo test -p alknet-call` succeeds
|
||||||
|
- [ ] `cargo clippy -p alknet-call` succeeds with no warnings
|
||||||
|
|
||||||
|
## References
|
||||||
|
|
||||||
|
- docs/architecture/crates/call/operation-registry.md — AccessControl (updated with the new check signature)
|
||||||
|
- docs/architecture/decisions/050-dynamic-resource-ownership-for-runtime-spawned-resources.md — ADR-050 §2, §4a
|
||||||
|
- crates/alknet-call/src/registry/spec.rs — current AccessControl::check implementation
|
||||||
|
- tasks/core/ownership-store-trait.md — the OwnershipProvider trait (dependency)
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
|
||||||
|
> This is the one-way door — the `check` signature change touches every
|
||||||
|
> call site and test. The key design property: backward compatibility when
|
||||||
|
> `ownership` is `None`. All existing call sites pass `(None, None)` in
|
||||||
|
> this task and behave exactly as before. The dispatch-path wiring
|
||||||
|
> (extracting `resource_id` from input and threading a real ownership
|
||||||
|
> provider) is the next task. This task is the signature + logic change;
|
||||||
|
> the next task is the wiring.
|
||||||
176
tasks/call/registry/dispatch-resource-id-extraction.md
Normal file
176
tasks/call/registry/dispatch-resource-id-extraction.md
Normal file
@@ -0,0 +1,176 @@
|
|||||||
|
---
|
||||||
|
id: call/registry/dispatch-resource-id-extraction
|
||||||
|
name: Wire dispatch path to extract resource_id from input and thread OwnershipProvider to AccessControl::check (ADR-050 §2a, §4a)
|
||||||
|
status: pending
|
||||||
|
depends_on: [call/registry/operation-spec-resource-id-path, call/registry/access-control-ownership-check]
|
||||||
|
scope: moderate
|
||||||
|
risk: medium
|
||||||
|
impact: component
|
||||||
|
level: implementation
|
||||||
|
---
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Wire the dispatch path to: (a) extract `resource_id` from the operation
|
||||||
|
input using `spec.resource_id_path`, and (b) thread an `OwnershipProvider`
|
||||||
|
to `AccessControl::check`. This is the task that makes the dynamic
|
||||||
|
ownership model actually work end-to-end. Per ADR-050 §2a and §4a.
|
||||||
|
|
||||||
|
### Where the wiring happens
|
||||||
|
|
||||||
|
The dispatch flow is:
|
||||||
|
|
||||||
|
```
|
||||||
|
dispatch() // protocol/dispatch.rs:220
|
||||||
|
→ extract input from payload // line 238
|
||||||
|
→ build_root_context(...) // line 246
|
||||||
|
→ registry.invoke(name, input, context) // line 261
|
||||||
|
→ look up registration // registration.rs:123
|
||||||
|
→ check visibility // registration.rs:128
|
||||||
|
→ check ACL: acl.check(...) // registration.rs:142 ← THIS IS WHERE resource_id + ownership go
|
||||||
|
→ call handler
|
||||||
|
```
|
||||||
|
|
||||||
|
`registry.invoke()` (in `registration.rs:116`) has access to both `input`
|
||||||
|
and `registration.spec` (which now has `resource_id_path`). The resource ID
|
||||||
|
extraction and ownership provider threading happen here.
|
||||||
|
|
||||||
|
### resource_id extraction (ADR-050 §2a)
|
||||||
|
|
||||||
|
In `invoke()` and `invoke_streaming()`, before the `check()` call:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
// Extract resource_id from input using spec.resource_id_path
|
||||||
|
let resource_id = registration.spec.resource_id_path
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|path| extract_json_pointer(&input, path));
|
||||||
|
```
|
||||||
|
|
||||||
|
`extract_json_pointer` parses a JSON pointer like `"$.containerId"` and
|
||||||
|
extracts the value at that path from the input `Value`. If the pointer is
|
||||||
|
`None`, or the path doesn't exist in the input, `resource_id` is `None`.
|
||||||
|
|
||||||
|
**JSON pointer parsing:** the `resource_id_path` uses the `$.field` syntax
|
||||||
|
(JSON Pointer with a leading `$`). Use the `serde_json` pointer API or a
|
||||||
|
small helper. `serde_json::Value::pointer()` takes a `/field` path — strip
|
||||||
|
the leading `$.` and replace with `/`, or use a simple helper that handles
|
||||||
|
the `$.` prefix. If the path is malformed or the field is missing,
|
||||||
|
`resource_id` is `None` (graceful — the check will deny if the spec
|
||||||
|
requires a resource_type but no ID is available).
|
||||||
|
|
||||||
|
### OwnershipProvider threading
|
||||||
|
|
||||||
|
The `OwnershipProvider` needs to be available in `invoke()`. Two options:
|
||||||
|
|
||||||
|
**Option A: thread it through `OperationContext`.** Add an
|
||||||
|
`ownership: Option<Arc<dyn OwnershipProvider>>` field to
|
||||||
|
`OperationContext`. `build_root_context` populates it from the
|
||||||
|
`CallAdapter` (which holds `Arc<dyn OwnershipProvider>`). `invoke()` reads
|
||||||
|
`context.ownership.as_deref()` and passes it to `check()`.
|
||||||
|
|
||||||
|
**Option B: hold it on the `OperationRegistry`.** The registry holds
|
||||||
|
`Option<Arc<dyn OwnershipProvider>>` and `invoke()` reads it directly.
|
||||||
|
|
||||||
|
**Recommendation: Option A (OperationContext).** It's consistent with how
|
||||||
|
`identity`, `capabilities`, and `env` are threaded — all on
|
||||||
|
`OperationContext`. The registry stays stateless (no new field); the
|
||||||
|
context carries the ownership provider the same way it carries the
|
||||||
|
identity provider's output. The assembly layer wires the ownership
|
||||||
|
provider into the `CallAdapter` at construction, and
|
||||||
|
`build_root_context` threads it onto the context.
|
||||||
|
|
||||||
|
### What changes
|
||||||
|
|
||||||
|
1. **`OperationContext` gains `ownership: Option<Arc<dyn OwnershipProvider>>`**
|
||||||
|
(in `operation-registry.md` / `operation-context.md` / the struct in
|
||||||
|
`registry/operation_context.rs` or wherever `OperationContext` is
|
||||||
|
defined). `None` when no ownership provider is wired (backward compat —
|
||||||
|
`check` falls back to static path).
|
||||||
|
|
||||||
|
2. **`build_root_context`** (in `protocol/dispatch.rs:148`) populates
|
||||||
|
`ownership` from `self.ownership_provider` (a new field on
|
||||||
|
`Dispatcher`/`CallAdapter`, set at construction by the assembly layer).
|
||||||
|
|
||||||
|
3. **`invoke()` and `invoke_streaming()`** (in `registration.rs`) extract
|
||||||
|
`resource_id` from `input` via `spec.resource_id_path`, then call
|
||||||
|
`acl.check(identity.as_ref(), resource_id.as_deref(),
|
||||||
|
context.ownership.as_deref())`.
|
||||||
|
|
||||||
|
4. **`invoke_with_policy()` in `connection.rs:311`** — the composition path
|
||||||
|
(internal calls via `OperationEnv::invoke`). Same pattern: extract
|
||||||
|
`resource_id` from `input` using the child op's `resource_id_path`,
|
||||||
|
thread the ownership provider from the parent context.
|
||||||
|
|
||||||
|
5. **`Dispatcher` / `CallAdapter`** gains an
|
||||||
|
`ownership_provider: Option<Arc<dyn OwnershipProvider>>` field, set at
|
||||||
|
construction. `None` by default (backward compat — deployments without
|
||||||
|
runtime-spawned resources don't wire one).
|
||||||
|
|
||||||
|
### What this task does NOT do
|
||||||
|
|
||||||
|
- Does NOT build a persistence adapter — the in-memory
|
||||||
|
`InMemoryOwnershipStore` from `core/ownership-store-trait` is the default.
|
||||||
|
- Does NOT build any handler that calls `record`/`revoke` — that's
|
||||||
|
downstream crate work (alknet-docker, etc.).
|
||||||
|
- Does NOT change `services/list` / `services/schema` / `services/list-peers`
|
||||||
|
— those stay on the `(None, None)` path (scope-gating only, no per-resource
|
||||||
|
ownership check at discovery time).
|
||||||
|
|
||||||
|
### Tests
|
||||||
|
|
||||||
|
- Integration test: an operation with `resource_type` + `resource_id_path`
|
||||||
|
+ an `OwnershipProvider` wired → `check` consults the provider → Allowed
|
||||||
|
when the provider says owns.
|
||||||
|
- Integration test: same setup but provider says not owns → Forbidden.
|
||||||
|
- Integration test: `resource_id_path` points to a field missing from input
|
||||||
|
→ `resource_id` is None → `list` path (owns_any) or Forbidden (if
|
||||||
|
resource_type requires a specific ID).
|
||||||
|
- Integration test: no ownership provider wired (`ownership: None` on
|
||||||
|
context) → static `Identity.resources` fallback (backward compat).
|
||||||
|
- Unit test: `extract_json_pointer` extracts `$.containerId` from
|
||||||
|
`{"containerId": "abc123"}` → `Some("abc123")`.
|
||||||
|
- Unit test: `extract_json_pointer` on missing field → `None`.
|
||||||
|
- Unit test: `resource_id_path: None` → `resource_id: None`.
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
|
||||||
|
- [ ] `OperationContext` has `ownership: Option<Arc<dyn OwnershipProvider>>` field
|
||||||
|
- [ ] `Dispatcher` / `CallAdapter` holds `ownership_provider: Option<Arc<dyn OwnershipProvider>>`, set at construction
|
||||||
|
- [ ] `build_root_context` populates `context.ownership` from the dispatcher's provider
|
||||||
|
- [ ] `invoke()` extracts `resource_id` from `input` via `spec.resource_id_path`, passes `(resource_id, context.ownership.as_deref())` to `check()`
|
||||||
|
- [ ] `invoke_streaming()` does the same
|
||||||
|
- [ ] `invoke_with_policy()` in `connection.rs` does the same for the composition path
|
||||||
|
- [ ] JSON pointer extraction helper handles `$.field` syntax
|
||||||
|
- [ ] Missing field in input → `resource_id: None` (graceful, not a panic)
|
||||||
|
- [ ] `resource_id_path: None` → `resource_id: None`
|
||||||
|
- [ ] No ownership provider wired → `check` falls back to static path (backward compat)
|
||||||
|
- [ ] Integration test: provider says owns → Allowed
|
||||||
|
- [ ] Integration test: provider says not owns → Forbidden
|
||||||
|
- [ ] Integration test: missing field → `list` path or Forbidden
|
||||||
|
- [ ] `cargo test -p alknet-call` succeeds
|
||||||
|
- [ ] `cargo clippy -p alknet-call` succeeds with no warnings
|
||||||
|
|
||||||
|
## References
|
||||||
|
|
||||||
|
- docs/architecture/crates/call/operation-registry.md — AccessControl (dispatch flow with resource_id extraction)
|
||||||
|
- docs/architecture/decisions/050-dynamic-resource-ownership-for-runtime-spawned-resources.md — ADR-050 §2a, §4a, §4d
|
||||||
|
- crates/alknet-call/src/protocol/dispatch.rs — dispatch() and build_root_context()
|
||||||
|
- crates/alknet-call/src/registry/registration.rs — invoke() and invoke_streaming() (the check() call sites)
|
||||||
|
- crates/alknet-call/src/protocol/connection.rs — invoke_with_policy() (composition path)
|
||||||
|
- tasks/call/registry/operation-spec-resource-id-path.md — the resource_id_path field (dependency)
|
||||||
|
- tasks/call/registry/access-control-ownership-check.md — the check() signature change (dependency)
|
||||||
|
- tasks/core/ownership-store-trait.md — the OwnershipProvider trait (dependency)
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
|
||||||
|
> This is the wiring task — it connects the pieces from the three
|
||||||
|
> dependency tasks into a working end-to-end flow. The key design choice is
|
||||||
|
> threading the ownership provider via `OperationContext` (consistent with
|
||||||
|
> how identity, capabilities, and env are threaded), not via the registry
|
||||||
|
> (which stays stateless). The assembly layer wires
|
||||||
|
> `Option<Arc<dyn OwnershipProvider>>` into the `CallAdapter` at
|
||||||
|
> construction; `None` is the default for deployments without
|
||||||
|
> runtime-spawned resources (backward compat). The JSON pointer extraction
|
||||||
|
> is a small helper — `$.containerId` → look up `containerId` in the input
|
||||||
|
> Value. Missing fields are graceful (None), not errors — the check denies
|
||||||
|
> if the spec requires a resource_type but no ID is available.
|
||||||
94
tasks/call/registry/operation-spec-resource-id-path.md
Normal file
94
tasks/call/registry/operation-spec-resource-id-path.md
Normal file
@@ -0,0 +1,94 @@
|
|||||||
|
---
|
||||||
|
id: call/registry/operation-spec-resource-id-path
|
||||||
|
name: Add resource_id_path field to OperationSpec (ADR-050 §2a)
|
||||||
|
status: pending
|
||||||
|
depends_on: []
|
||||||
|
scope: single
|
||||||
|
risk: low
|
||||||
|
impact: component
|
||||||
|
level: implementation
|
||||||
|
---
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Add a `resource_id_path: Option<String>` field to `OperationSpec`. This is
|
||||||
|
a JSON pointer into the operation input that tells the dispatcher where to
|
||||||
|
find the resource ID for runtime-spawned resource authorization. Per ADR-050
|
||||||
|
§2a.
|
||||||
|
|
||||||
|
### The field
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub struct OperationSpec {
|
||||||
|
pub name: String,
|
||||||
|
pub namespace: String,
|
||||||
|
pub op_type: OperationType,
|
||||||
|
pub visibility: Visibility,
|
||||||
|
pub input_schema: Value,
|
||||||
|
pub output_schema: Value,
|
||||||
|
pub error_schemas: Vec<ErrorDefinition>,
|
||||||
|
pub access_control: AccessControl,
|
||||||
|
/// JSON pointer into the input for the resource ID, when
|
||||||
|
/// `access_control.resource_type` is set and the operation targets a
|
||||||
|
/// specific runtime-spawned resource (ADR-050). e.g., `"$.containerId"`
|
||||||
|
/// for `docker/container/exec`. Absent for no-specific-resource
|
||||||
|
/// operations (the `list` case). `None` for operations with no
|
||||||
|
/// `resource_type` or with static resource sets.
|
||||||
|
pub resource_id_path: Option<String>,
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Construction
|
||||||
|
|
||||||
|
`OperationSpec::new(...)` currently takes 7 arguments (name, op_type,
|
||||||
|
visibility, input_schema, output_schema, error_schemas, access_control).
|
||||||
|
This task adds `resource_id_path` as an 8th argument. Since all
|
||||||
|
construction sites need to update, consider whether a builder pattern is
|
||||||
|
worth introducing — but for now, add it as the last positional argument
|
||||||
|
(defaulting to `None` at call sites that don't need it).
|
||||||
|
|
||||||
|
### What this task does NOT do
|
||||||
|
|
||||||
|
- Does NOT change `AccessControl::check` — that's
|
||||||
|
`call/registry/access-control-ownership-check`, which depends on
|
||||||
|
`core/ownership-store-trait`.
|
||||||
|
- Does NOT extract the resource ID from input or pass it to `check` —
|
||||||
|
that's `call/registry/dispatch-resource-id-extraction`, which depends on
|
||||||
|
this task.
|
||||||
|
- This task only adds the field to the struct, updates `new()`, and updates
|
||||||
|
all existing construction sites + tests.
|
||||||
|
|
||||||
|
### Existing construction sites
|
||||||
|
|
||||||
|
Search for `OperationSpec::new(` across the codebase — every call site
|
||||||
|
needs the new argument added. Most will pass `None` (no runtime-spawned
|
||||||
|
resources). The existing tests in `spec.rs` construct `OperationSpec`
|
||||||
|
without `resource_id_path` — they all need the `None` argument added.
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
|
||||||
|
- [ ] `OperationSpec` struct has `resource_id_path: Option<String>` field
|
||||||
|
- [ ] `OperationSpec::new(...)` takes `resource_id_path` as the 8th argument
|
||||||
|
- [ ] All existing `OperationSpec::new(...)` call sites updated (most pass `None`)
|
||||||
|
- [ ] All existing tests that construct `OperationSpec` updated
|
||||||
|
- [ ] Existing tests still pass (no semantic change — `None` means "no resource ID extraction")
|
||||||
|
- [ ] Unit test: `resource_id_path` is `None` by default when not specified
|
||||||
|
- [ ] `cargo test -p alknet-call` succeeds
|
||||||
|
- [ ] `cargo clippy -p alknet-call` succeeds with no warnings
|
||||||
|
|
||||||
|
## References
|
||||||
|
|
||||||
|
- docs/architecture/crates/call/operation-registry.md — OperationSpec (updated with `resource_id_path`)
|
||||||
|
- docs/architecture/decisions/050-dynamic-resource-ownership-for-runtime-spawned-resources.md — ADR-050 §2a
|
||||||
|
- crates/alknet-call/src/registry/spec.rs — current OperationSpec struct
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
|
||||||
|
> The fit with JSON Schema is load-bearing: `input_schema` is already a
|
||||||
|
> JSON Schema, so `resource_id_path` is a pointer *within* an existing
|
||||||
|
> schema on the same spec. The `OperationSpec` becomes fully
|
||||||
|
> self-describing for authorization — what resource type, what action,
|
||||||
|
> and *which input field* drives the resource lookup. This is a single
|
||||||
|
> field addition with no semantic change — existing call sites pass
|
||||||
|
> `None` and behave exactly as before. The field is consumed by the
|
||||||
|
> dispatch path in `call/registry/dispatch-resource-id-extraction`.
|
||||||
199
tasks/core/ownership-store-trait.md
Normal file
199
tasks/core/ownership-store-trait.md
Normal file
@@ -0,0 +1,199 @@
|
|||||||
|
---
|
||||||
|
id: core/ownership-store-trait
|
||||||
|
name: Add OwnershipProvider (sync read) + OwnershipStore (async write) traits and InMemoryOwnershipStore (ADR-050)
|
||||||
|
status: pending
|
||||||
|
depends_on: []
|
||||||
|
scope: moderate
|
||||||
|
risk: low
|
||||||
|
impact: component
|
||||||
|
level: implementation
|
||||||
|
---
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Add the `OwnershipProvider` (sync read trait) and `OwnershipStore` (async
|
||||||
|
write trait) for runtime-spawned resource ownership, plus an
|
||||||
|
`InMemoryOwnershipStore` default adapter. This is the fourth instance of
|
||||||
|
the repo/adapter pattern (ADR-033), alongside `IdentityProvider` (ADR-004),
|
||||||
|
`IdentityStore` (ADR-035), and `CredentialStore` (ADR-031). Per ADR-050.
|
||||||
|
|
||||||
|
### Why this exists
|
||||||
|
|
||||||
|
Runtime-spawned resources (containers, TTYs, workspace processes) have
|
||||||
|
derived ownership: whoever spawned the resource owns it. The static
|
||||||
|
`Identity.resources` model can't represent this — the resource didn't exist
|
||||||
|
when the identity was resolved. `AccessControl::check` (in alknet-call) will
|
||||||
|
consult `OwnershipProvider` at check time to answer "does this identity own
|
||||||
|
this specific resource?"
|
||||||
|
|
||||||
|
### Module placement
|
||||||
|
|
||||||
|
Create a new `ownership.rs` module in `alknet-core/src/`, re-export from
|
||||||
|
`lib.rs`. Follow the same pattern as `store.rs` (which holds
|
||||||
|
`CredentialStore` + `InMemoryCredentialStore`).
|
||||||
|
|
||||||
|
### OwnershipProvider trait (read side, sync)
|
||||||
|
|
||||||
|
```rust
|
||||||
|
/// Read side: consulted by AccessControl::check on the dispatch hot path.
|
||||||
|
/// Sync — called in the dispatch loop, no .await.
|
||||||
|
pub trait OwnershipProvider: Send + Sync + 'static {
|
||||||
|
/// Does `identity` own `resource_type/resource_id` with `action`?
|
||||||
|
/// Called when AccessControl has resource_type + resource_action set
|
||||||
|
/// and the dispatcher has extracted resource_id from the input via
|
||||||
|
/// OperationSpec.resource_id_path (ADR-050 §2a).
|
||||||
|
fn owns(
|
||||||
|
&self,
|
||||||
|
identity: &Identity,
|
||||||
|
resource_type: &str,
|
||||||
|
resource_id: &str,
|
||||||
|
action: &str,
|
||||||
|
) -> bool;
|
||||||
|
|
||||||
|
/// What resources of `resource_type` does `identity` own?
|
||||||
|
/// Called for the `list` case (resource_type set, resource_id_path
|
||||||
|
/// absent) — the result-filter path (ADR-050 §4a). Returns the set of
|
||||||
|
/// resource IDs the caller owns, for the handler to filter against.
|
||||||
|
fn owned_resources(
|
||||||
|
&self,
|
||||||
|
identity: &Identity,
|
||||||
|
resource_type: &str,
|
||||||
|
) -> Vec<String>;
|
||||||
|
|
||||||
|
/// Does `identity` own *any* resource of `resource_type`?
|
||||||
|
/// Called for the `list` case — the scope-gate path (ADR-050 §4a).
|
||||||
|
fn owns_any(
|
||||||
|
&self,
|
||||||
|
identity: &Identity,
|
||||||
|
resource_type: &str,
|
||||||
|
) -> bool;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### OwnershipStore trait (write side, async)
|
||||||
|
|
||||||
|
```rust
|
||||||
|
/// Write side: called by the handler that manages the resource lifecycle.
|
||||||
|
/// Async — not on the dispatch hot path. The handler calls `record` on
|
||||||
|
/// spawn and `revoke` on teardown (ADR-050 §4b — handler-driven, not a
|
||||||
|
/// reaper).
|
||||||
|
#[async_trait]
|
||||||
|
pub trait OwnershipStore: Send + Sync + 'static {
|
||||||
|
/// Record that `identity` spawned `resource_type/resource_id`.
|
||||||
|
async fn record(
|
||||||
|
&self,
|
||||||
|
identity: &Identity,
|
||||||
|
resource_type: &str,
|
||||||
|
resource_id: &str,
|
||||||
|
) -> Result<(), OwnershipError>;
|
||||||
|
|
||||||
|
/// Revoke ownership of `resource_type/resource_id`.
|
||||||
|
/// Called by the handler on resource teardown (ADR-050 §4b).
|
||||||
|
async fn revoke(
|
||||||
|
&self,
|
||||||
|
resource_type: &str,
|
||||||
|
resource_id: &str,
|
||||||
|
) -> Result<(), OwnershipError>;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Note: the ADR-050 sketch shows `record`/`revoke` taking `&mut self`. The
|
||||||
|
in-memory implementation should use interior mutability (`RwLock` or
|
||||||
|
`Mutex`, same as `InMemoryCredentialStore` which takes `&self` for its
|
||||||
|
async methods). The trait takes `&self` so it can be shared as
|
||||||
|
`Arc<dyn OwnershipStore>`.
|
||||||
|
|
||||||
|
### OwnershipError
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[non_exhaustive]
|
||||||
|
#[derive(Debug, thiserror::Error)]
|
||||||
|
pub enum OwnershipError {
|
||||||
|
#[error("backend error: {message}")]
|
||||||
|
Backend { message: String },
|
||||||
|
#[error("not found: {entity}")]
|
||||||
|
NotFound { entity: String },
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Follow the same shape as `StoreError` in `store.rs`. `#[non_exhaustive]`
|
||||||
|
so future variants (e.g., concurrency conflict) can be added without
|
||||||
|
breaking consumers.
|
||||||
|
|
||||||
|
### InMemoryOwnershipStore
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub struct InMemoryOwnershipStore {
|
||||||
|
// Map: (resource_type, resource_id) → owner Identity
|
||||||
|
inner: RwLock<HashMap<(String, String), Identity>>,
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- `owns`: look up `(resource_type, resource_id)`, compare owner's `id`
|
||||||
|
to `identity.id`, check that the action is... actually, the ownership
|
||||||
|
store records *who owns* a resource, not *what actions they can perform*.
|
||||||
|
The `action` parameter in `owns` is the `resource_action` from
|
||||||
|
`AccessControl` — it's passed for future use (per-action ownership
|
||||||
|
grants), but the base model is "owner can do anything they own." For
|
||||||
|
now, `owns` returns `true` if the identity owns the resource, regardless
|
||||||
|
of action. The action parameter is accepted but not gated on — this
|
||||||
|
preserves the door for per-action grants without building them now.
|
||||||
|
- `owned_resources`: iterate the map, return IDs where the owner's `id`
|
||||||
|
matches.
|
||||||
|
- `owns_any`: iterate the map, return `true` if any entry matches the
|
||||||
|
identity + resource_type.
|
||||||
|
- `record`: insert `(resource_type, resource_id) → identity`.
|
||||||
|
- `revoke`: remove `(resource_type, resource_id)`.
|
||||||
|
|
||||||
|
### What this task does NOT do
|
||||||
|
|
||||||
|
- Does NOT change `AccessControl::check` — that's
|
||||||
|
`call/registry/access-control-ownership-check`, which depends on this
|
||||||
|
task.
|
||||||
|
- Does NOT add `resource_id_path` to `OperationSpec` — that's
|
||||||
|
`call/registry/operation-spec-resource-id-path`, which is independent.
|
||||||
|
- Does NOT build a persistence adapter (SQLite/honker-backed) — that's
|
||||||
|
additive, built when a concrete use case forces it (ADR-050 §1).
|
||||||
|
- Does NOT wire the ownership provider into the dispatch path — that's
|
||||||
|
`call/registry/dispatch-resource-id-extraction`, which depends on this
|
||||||
|
task and the two call tasks.
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
|
||||||
|
- [ ] `OwnershipProvider` trait with `owns`, `owned_resources`, `owns_any` (all sync)
|
||||||
|
- [ ] `OwnershipStore` trait with `record`, `revoke` (both async, `#[async_trait]`)
|
||||||
|
- [ ] `OwnershipError` enum (`#[non_exhaustive]`, `thiserror::Error`) with `Backend` + `NotFound` variants
|
||||||
|
- [ ] `InMemoryOwnershipStore` implements both `OwnershipProvider` + `OwnershipStore`
|
||||||
|
- [ ] `InMemoryOwnershipStore` uses interior mutability (`RwLock`, same as `InMemoryCredentialStore`)
|
||||||
|
- [ ] `owns` returns true if identity owns the resource (action accepted but not gated — base model is owner-can-do-anything)
|
||||||
|
- [ ] `owned_resources` returns the set of resource IDs the identity owns for a given type
|
||||||
|
- [ ] `owns_any` returns true if identity owns at least one resource of the type
|
||||||
|
- [ ] `record` inserts ownership; `revoke` removes it
|
||||||
|
- [ ] New `ownership.rs` module in `alknet-core/src/`
|
||||||
|
- [ ] Re-exported from `lib.rs`: `pub use ownership::{OwnershipProvider, OwnershipStore, InMemoryOwnershipStore, OwnershipError};`
|
||||||
|
- [ ] Unit tests: record → owns → revoke → not owns round-trip
|
||||||
|
- [ ] Unit test: owned_resources returns correct set for an owner with multiple resources
|
||||||
|
- [ ] Unit test: owns_any returns false for an owner with no resources of that type
|
||||||
|
- [ ] Unit test: revoke on non-existent resource is a no-op (or NotFound — pick one and document)
|
||||||
|
- [ ] `cargo test -p alknet-core` succeeds
|
||||||
|
- [ ] `cargo clippy -p alknet-core` succeeds with no warnings
|
||||||
|
|
||||||
|
## References
|
||||||
|
|
||||||
|
- docs/architecture/crates/core/auth.md — "Ownership Provider and Store (ADR-050)" section
|
||||||
|
- docs/architecture/decisions/050-dynamic-resource-ownership-for-runtime-spawned-resources.md — ADR-050 (the full decision)
|
||||||
|
- docs/architecture/decisions/033-storage-boundary-and-repo-adapter-pattern.md — ADR-033 (the pattern)
|
||||||
|
- crates/alknet-core/src/store.rs — `CredentialStore` + `InMemoryCredentialStore` (the pattern to follow)
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
|
||||||
|
> The read/write split mirrors ADR-035: `OwnershipProvider` (read, sync) is
|
||||||
|
> the trait the dispatch path depends on; `OwnershipStore` (write, async)
|
||||||
|
> is the trait the handler lifecycle calls. The in-memory default
|
||||||
|
> implements both. The `owns` method accepts an `action` parameter but
|
||||||
|
> doesn't gate on it — the base model is "owner can do anything they own."
|
||||||
|
> Per-action grants are a future extension (additive, not needed now). The
|
||||||
|
> `action` parameter preserves the door without building the mechanism. A
|
||||||
|
> persistence adapter (SQLite/honker-backed) is not built in this sync —
|
||||||
|
> ownership is runtime state, meaningless across restarts; the in-memory
|
||||||
|
> default is sufficient for the docker/runner cases.
|
||||||
Reference in New Issue
Block a user