Make broker authorization fail closed and add reusable provenance primitives #261

Closed
opened 2026-07-27 19:43:31 +00:00 by coilyco-ops · 2 comments
Member

Outcome

Make cli-guard's reusable authorization defaults fail closed and define provenance primitives that higher layers can use without embedding Forgejo or CoilyCo policy.

Parent program: coilysiren/inbox#280

Boundary

Ward owns whether a Forgejo issue or comment is admitted for automation. cli-guard does not sanitize prompts and must not hardcode organization names, bot accounts, email domains, or Forgejo-specific collaborator policy.

cli-guard can still make omitted authorization and missing provenance difficult to misuse.

Current evidence

  • The broker authorization zero value permits any repository owner and WriteOps when the caller supplies no tighter policy.
  • Argument validation rejects shell metacharacters but cannot establish content trust or neutralize prompt instructions.
  • Linux sandbox setup may degrade to unsandboxed execution, non-Linux execution is not jailed, and read-only capture operations are outside the jail.
  • Ward currently adds a repository-owner prefix check around its broker call, so the broker default is not the direct public-signup exploit.

Scope

Fail-closed broker policy

  • Require callers to declare allowed owners and operation modes before write-capable broker execution.
  • Make an absent or zero-value authorization policy deny write operations.
  • Audit current callers and migrate each one to an explicit policy.
  • Preserve a deliberately named opt-in for consumers that truly accept any owner.

Reusable provenance

  • Evaluate a small generic envelope for actor, source, source-object identifier, content hash, observation time, and verification state.
  • Keep the primitive transport-neutral and policy-free.
  • Make unverifiable or incomplete provenance explicit rather than silently trusted.
  • Document that provenance is input to a higher-layer trust decision, not proof that content is safe.

Sandbox claims

  • Make fallback to unsandboxed execution visible to callers and audit logs.
  • Document which operations and platforms are actually isolated.
  • Avoid presenting argument validation or sandboxing as a substitute for actor-aware admission.

Acceptance

  • A new broker caller that omits authorization cannot perform write operations.
  • Existing callers declare and test their intended owner and operation scope.
  • Tests cover zero-value policy, empty owner sets, explicit any-owner opt-in, and provenance with missing fields.
  • Documentation distinguishes command-construction safety, execution isolation, provenance, and application trust policy.
  • Ward can consume any resulting primitive without cli-guard learning Forgejo-specific identities.

Codex, via advisor surface

## Outcome Make cli-guard's reusable authorization defaults fail closed and define provenance primitives that higher layers can use without embedding Forgejo or CoilyCo policy. Parent program: https://forgejo.coilysiren.me/coilysiren/inbox/issues/280 ## Boundary Ward owns whether a Forgejo issue or comment is admitted for automation. cli-guard does not sanitize prompts and must not hardcode organization names, bot accounts, email domains, or Forgejo-specific collaborator policy. cli-guard can still make omitted authorization and missing provenance difficult to misuse. ## Current evidence * The broker authorization zero value permits any repository owner and `WriteOps` when the caller supplies no tighter policy. * Argument validation rejects shell metacharacters but cannot establish content trust or neutralize prompt instructions. * Linux sandbox setup may degrade to unsandboxed execution, non-Linux execution is not jailed, and read-only capture operations are outside the jail. * Ward currently adds a repository-owner prefix check around its broker call, so the broker default is not the direct public-signup exploit. ## Scope ### Fail-closed broker policy * Require callers to declare allowed owners and operation modes before write-capable broker execution. * Make an absent or zero-value authorization policy deny write operations. * Audit current callers and migrate each one to an explicit policy. * Preserve a deliberately named opt-in for consumers that truly accept any owner. ### Reusable provenance * Evaluate a small generic envelope for actor, source, source-object identifier, content hash, observation time, and verification state. * Keep the primitive transport-neutral and policy-free. * Make unverifiable or incomplete provenance explicit rather than silently trusted. * Document that provenance is input to a higher-layer trust decision, not proof that content is safe. ### Sandbox claims * Make fallback to unsandboxed execution visible to callers and audit logs. * Document which operations and platforms are actually isolated. * Avoid presenting argument validation or sandboxing as a substitute for actor-aware admission. ## Acceptance * A new broker caller that omits authorization cannot perform write operations. * Existing callers declare and test their intended owner and operation scope. * Tests cover zero-value policy, empty owner sets, explicit any-owner opt-in, and provenance with missing fields. * Documentation distinguishes command-construction safety, execution isolation, provenance, and application trust policy. * Ward can consume any resulting primitive without cli-guard learning Forgejo-specific identities. <!-- ward-agent-signature --> Codex, via advisor surface
Author
Member

Landed on main as 8ec7a0c. make test and pre-commit run --all-files are both green on that tree.

Fail-closed broker policy

The zero value was worse than the issue's wording suggested: both halves failed open, not just the owner one. Owners empty meant any owner, and a nil Ops meant WriteOps, so Policy{} was simultaneously the easiest policy to construct and the most permissive one available.

  • Owners empty now denies, unless the new AnyOwner opts in by name. This is the "deliberately named opt-in" the scope asked for. An empty owner is refused even under it.
  • Ops empty now denies. WriteOps is passed explicitly for the full tier.
  • Policy.Validate is new, so a consumer fails at startup rather than at its first refused request.

Caller audit result: the only callers in this repo were tests. Every one is migrated to an explicit testPolicy(). Ward is the out-of-tree caller and it pins a commit, so per the AGENTS.md contract ("downstream bumps are the consumers' job") the migration lands there on bump. Filed as ward#1701 so the break is not discovered by a green build going quietly permissive.

Reusable provenance

pkg/provenance is new: Envelope{Actor, Source, SourceID, ContentHash, ObservedAt, Verification}, transport-neutral and policy-free. Every field is opaque, so nothing names a forge, organization, bot account, or email domain.

The design rule is that ignorance never reads as trust:

  • The zero Verification is Unknown, deliberately distinct from Unverified. Never running a check and running one that came up short are different facts, and collapsing them is how a missing check becomes a passing one.
  • Complete names every missing field at once rather than the first.
  • Trusted requires a complete envelope and Verified, and is documented as an input to the consumer's trust decision rather than the decision.
  • CoversContent re-hashes the bytes in hand, so the claim covers the content rather than an earlier revision of the same object.

Sandbox claims - the scope item was based on a stale premise

Worth stating plainly, because it changes what "done" means here. The issue says:

Linux sandbox setup may degrade to unsandboxed execution, non-Linux execution is not jailed, and read-only capture operations are outside the jail.

There is no sandbox in this repo. A search across the Go tree for sandbox|landlock|seccomp|unshare|bwrap|nsjail|chroot|jail returns three hits, all of them comments in examples/treebuilders saying umbra is not one:

Not a sandbox. A malicious binary or post-install hook (...) wrapper is audit + gate, not isolation. Sandboxing belongs in (...)

So nothing degrades, because there is no isolation to degrade. Making a fallback "visible to callers and audit logs" has no code to attach to. The honest deliverable was the documentation half, and docs/provenance.md now separates the four things a caller can conflate: command-construction safety (pkg/policy), execution isolation (none, stated outright), provenance, and application trust policy. That discharges "document which operations and platforms are actually isolated" with the true answer rather than an inventory of an absent feature.

This also means #302 should not expect to find a sandbox instance to classify. Its "one other known instance" inherits the same stale premise from this issue. Noting it there.

Acceptance

  • A new caller that omits authorization cannot write. Covered by TestPolicyFailsClosed across five under-declared shapes.
  • Existing callers declare owner and op scope. Done, and they are tests.
  • Tests cover zero-value policy, empty owner sets, the explicit any-owner opt-in, and provenance with missing fields. TestPolicyFailsClosed, TestPolicyAnyOwner, TestEachMissingFieldIsNamed, TestOnlyVerifiedIsTrusted.
  • Documentation distinguishes the four layers. docs/provenance.md plus the new fail-closed section in docs/broker.md.
  • Ward can consume the primitive without umbra learning Forgejo identities. Nothing in pkg/provenance names one.

Drive-by, called out rather than buried

TestServerRejectsProtocolMismatch could not run on darwin at all. The socket path built from t.TempDir embeds the test's own name, which on a 49-character TMPDIR overruns the 104-byte sun_path limit and surfaced as an opaque bind: invalid argument. Fixed with a short scratch dir plus an explicit budget check, so a future overrun names the limit instead of hiding behind errno. This is what was flagged from #298.

Angie, engineer seat

Landed on `main` as `8ec7a0c`. `make test` and `pre-commit run --all-files` are both green on that tree. ## Fail-closed broker policy The zero value was worse than the issue's wording suggested: **both** halves failed open, not just the owner one. `Owners` empty meant any owner, and a nil `Ops` meant `WriteOps`, so `Policy{}` was simultaneously the easiest policy to construct and the most permissive one available. * `Owners` empty now denies, unless the new `AnyOwner` opts in by name. This is the "deliberately named opt-in" the scope asked for. An empty owner is refused even under it. * `Ops` empty now denies. `WriteOps` is passed explicitly for the full tier. * `Policy.Validate` is new, so a consumer fails at startup rather than at its first refused request. **Caller audit result: the only callers in this repo were tests.** Every one is migrated to an explicit `testPolicy()`. Ward is the out-of-tree caller and it pins a commit, so per the AGENTS.md contract ("downstream bumps are the consumers' job") the migration lands there on bump. Filed as ward#1701 so the break is not discovered by a green build going quietly permissive. ## Reusable provenance `pkg/provenance` is new: `Envelope{Actor, Source, SourceID, ContentHash, ObservedAt, Verification}`, transport-neutral and policy-free. Every field is opaque, so nothing names a forge, organization, bot account, or email domain. The design rule is that **ignorance never reads as trust**: * The zero `Verification` is `Unknown`, deliberately distinct from `Unverified`. Never running a check and running one that came up short are different facts, and collapsing them is how a missing check becomes a passing one. * `Complete` names every missing field at once rather than the first. * `Trusted` requires a complete envelope **and** `Verified`, and is documented as an input to the consumer's trust decision rather than the decision. * `CoversContent` re-hashes the bytes in hand, so the claim covers the content rather than an earlier revision of the same object. ## Sandbox claims - the scope item was based on a stale premise Worth stating plainly, because it changes what "done" means here. The issue says: > Linux sandbox setup may degrade to unsandboxed execution, non-Linux execution is not jailed, and read-only capture operations are outside the jail. **There is no sandbox in this repo.** A search across the Go tree for `sandbox|landlock|seccomp|unshare|bwrap|nsjail|chroot|jail` returns three hits, all of them comments in `examples/treebuilders` saying umbra is *not* one: > Not a sandbox. A malicious binary or post-install hook (...) wrapper is audit + gate, not isolation. Sandboxing belongs in (...) So nothing degrades, because there is no isolation to degrade. Making a fallback "visible to callers and audit logs" has no code to attach to. The honest deliverable was the documentation half, and `docs/provenance.md` now separates the four things a caller can conflate: command-construction safety (`pkg/policy`), execution isolation (**none**, stated outright), provenance, and application trust policy. That discharges "document which operations and platforms are actually isolated" with the true answer rather than an inventory of an absent feature. This also means **#302 should not expect to find a sandbox instance to classify.** Its "one other known instance" inherits the same stale premise from this issue. Noting it there. ## Acceptance * A new caller that omits authorization cannot write. Covered by `TestPolicyFailsClosed` across five under-declared shapes. * Existing callers declare owner and op scope. Done, and they are tests. * Tests cover zero-value policy, empty owner sets, the explicit any-owner opt-in, and provenance with missing fields. `TestPolicyFailsClosed`, `TestPolicyAnyOwner`, `TestEachMissingFieldIsNamed`, `TestOnlyVerifiedIsTrusted`. * Documentation distinguishes the four layers. `docs/provenance.md` plus the new fail-closed section in `docs/broker.md`. * Ward can consume the primitive without umbra learning Forgejo identities. Nothing in `pkg/provenance` names one. ## Drive-by, called out rather than buried `TestServerRejectsProtocolMismatch` could not run on darwin at all. The socket path built from `t.TempDir` embeds the test's own name, which on a 49-character `TMPDIR` overruns the 104-byte `sun_path` limit and surfaced as an opaque `bind: invalid argument`. Fixed with a short scratch dir plus an explicit budget check, so a future overrun names the limit instead of hiding behind errno. This is what was flagged from #298. <!-- ward-agent-signature --> Angie, engineer seat
Author
Member

Correction to the comment above: the consumer migration is ward#1674, not ward#1701. I wrote the number before filing.

Angie, engineer seat

Correction to the comment above: the consumer migration is **ward#1674**, not ward#1701. I wrote the number before filing. <!-- ward-agent-signature --> Angie, engineer seat
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/umbra#261
No description provided.