Skip to content

A policy's resource = "node" rules were compiled by nobody, so a denied kind ran - #77

Merged
Shashankss1205 merged 2 commits into
mainfrom
fix/issue-66
Aug 3, 2026
Merged

A policy's resource = "node" rules were compiled by nobody, so a denied kind ran#77
Shashankss1205 merged 2 commits into
mainfrom
fix/issue-66

Conversation

@Shashankss1205

Copy link
Copy Markdown
Collaborator

Fixes #66.

I took the first option in the issue — enforce the rules — rather than
refusing to compile the document. The enforcement path already existed on the
edge side and the node side needed the same three pieces (a policy object, a
compiler, a check), so the change is symmetric with code that is already there
rather than a new mechanism, and it leaves the operator with the rule they
wrote instead of an error telling them to delete it.

What was wrong

PolicyEngine.edge_policy() compiled the edge half of a document and nothing
compiled the other one. AdmissionChecker gated node kinds on NodeRegistry
membership alone, and check_node — correct, tested, advertised in the engine's
module docstring as the answer to "may this node run?" — had no runtime caller
anywhere in grapharc/. So a valid, non-refused deny rule over a node kind
meant nothing, on the documented path: grapharc/policy/example.toml ships
no-shell-nodes as the example of governing what may run.

What changed

  • NodePolicy / NodeRule (grapharc/planner/admission.py) beside
    EdgePolicy / EdgeRule: same tiered semantics (every deny before every ask
    before every allow, first match within a tier, unmatched takes the default),
    match is an fnmatch over a registry kind, and reason is carried from
    the document so a refusal can quote it.
  • PolicyEngine.node_policy(tenant=…) compiles the node half exactly as
    edge_policy() compiles the edge half, including the undeclared-tenant
    denial. A test pins it to check_node across a kind × tenant matrix, as the
    edge one already was.
  • AdmissionChecker(node_policy=…) checks every proposed node in every
    scope, keyed on kind. Refusals are policy/node_denied and
    policy/node_needs_approval — under the existing POLICY check, so a planner
    replans against them the same way it does edge_denied, and an ask-only
    result still reports NEEDS_APPROVAL.
  • GatePolicy (grapharc/cli/plan.py) carries both halves of a document
    together; resolve_edge_policy becomes resolve_policy, and compile_policy
    is the single place a document becomes admission's objects — so --policy, a
    cached generated policy and a freshly generated one cannot be read three
    different ways. grapharc plan and grapharc run pass both halves through.
  • The banner counts both: (tenant 'default', 1 edge rule(s), 2 node rule(s)).

The one judgement call

node_policy() is faithful to check_node, so a document with no node
rules and default = "deny" compiles to a policy that denies every kind. That
is the right answer for the API and the wrong reading of an operator's intent,
so the CLI compiles the node half only when the document declares at least one
node rule
. Saying nothing about nodes is not the same statement as denying
all of them, and the registry — an allowlist with no wildcard — is still the
gate in that case. node_policy=None means exactly that, and is what every
existing caller (and every edge-only policy document) keeps.

Before / after — the issue's exact repro

Before, with the issue's nodepolicy.toml verbatim:

policy    : nodepolicy.toml (tenant 'default', 1 edge rule(s))  [flag-or-config]
   round 1: admitted  nodes=2 executed=True
state     : notes=['triage ran', 'deploy ran', 'triage ran', 'patch ran', 'verify ran']

After, same file:

policy    : nodepolicy.toml (tenant 'default', 1 edge rule(s), 1 node rule(s))
stopped   : planning_failed
   round 1: rejected  nodes=2 executed=False  rejected: node_denied, node_denied
   round 2: rejected  nodes=3 executed=False  rejected: node_denied, node_denied, node_denied
state     : notes=[]

deploy ran is gone, and the deploy rejection quotes the rule:

the node policy denies this kind: kind 'deploy' (proposed as 'n'): deploying from a plan is never permitted

Every kind is refused there because that document says default = "deny" and
names only deploy, so triage matches no node rule and takes the default —
which is exactly what engine.check_node("triage") has always answered for it.
Add the catch-all its author's default needs and the intended behaviour is what
you get:

[[rule]]
id = "other-nodes-run"
resource = "node"
match = "*"
effect = "allow"
   round 1: rejected  nodes=2 executed=False  rejected: node_denied
   round 2: admitted  nodes=3 executed=True
state     : notes=['triage ran', 'patch ran', 'verify ran']

example.toml

Its node rules are enforced rather than removed, verified by driving the shipped
document through the gate (test_the_shipped_examples_node_rules_reach_the_admission_gate):
a shell_exec kind is refused with "a shell node is an unbounded tool" and
summarise is admitted. The section gained a comment saying the rules match a
registry kind and reach the checker.

Docs

  • docs/cookbook/05-governance.md: new executed recipe "How do I stop a node
    kind from running, from the document?"; the bridge section no longer claims
    there is no shipped compiler; limitation 9 rewritten; limitation 3 now covers
    node approvals. The cookbook's policy.toml gained a reason on
    no-shell-nodes so the recipe shows a quoted refusal — the two printed policy
    digests were re-run and updated with it.
  • README's policy paragraph, grapharc.policy and grapharc.planner.admission
    docstrings, ROADMAP §7.5, CHANGELOG.

Verification

  • python -m pytest: 1802 passed, 12 deselected — twice (default random
    ordering, and -p no:randomly). No failures at all, including the SIGALRM
    timing tests.
  • ruff check .: clean.
  • 22 new tests: node deny/ask/allow and tiering, kind-not-name in both
    directions, nested scopes, reason quoting, node_policy=None leaving the
    registry as the only gate, engine/compiled equivalence across tenants, the
    shipped example through the gate, the CLI repro end to end (including that
    deploy ran is absent and the trace carries policy/node_denied), and an
    operator's node rules added to a cached generated policy being honoured.

Compatibility

grapharc.cli.plan.resolve_edge_policy is now resolve_policy and returns a
GatePolicy; resolve_or_generate_policy's first element is that object rather
than a bare EdgePolicy (.edge is the old value). A registry module supplying
its own build_loop now receives node_policy= — both shipped ones accept it,
and a third-party one would need the keyword added.

🤖 Generated with Claude Code

Shashankss1205 and others added 2 commits August 4, 2026 00:51
…nied kind ran

`PolicyEngine.edge_policy()` compiled the edge half of a document and nothing
compiled the other one. `AdmissionChecker` gated node kinds on `NodeRegistry`
membership alone, and `check_node` — correct, tested, and advertised in the
engine's own module docstring as the answer to "may this node run?" — had no
runtime caller anywhere in `grapharc/`. A written, valid, non-refused `deny`
rule over a node kind therefore meant nothing at all:

    $ grapharc plan "fix the outage" --policy nodepolicy.toml
    policy    : nodepolicy.toml (tenant 'default', 1 edge rule(s))
       round 1: admitted  nodes=2 executed=True
    state     : notes=['triage ran', 'deploy ran', ...]

Fail-open, silent, and on the documented path: `grapharc/policy/example.toml`
ships `no-shell-nodes` as the canonical example of governing what may run, so an
operator who copied the shipped example got a policy that denied nothing. The
only hint that half the file had been discarded was `1 edge rule(s)` in a line
that reads as a summary rather than a warning.

**Enforced, not merely refused at load.** `NodePolicy`/`NodeRule` sit beside
`EdgePolicy`/`EdgeRule` with the same tiered semantics — every deny before every
ask before every allow, first match within a tier, unmatched takes the default —
and `PolicyEngine.node_policy(tenant=…)` compiles the node half exactly as
`edge_policy()` compiles the edge half. A test pins the compiled object to
`check_node` across a kind x tenant matrix, as the edge one already was.

`AdmissionChecker(node_policy=…)` consults it for every proposed node, in every
scope, keyed on the registry `kind` like every other node decision — so renaming
a denied instance launders nothing and naming an instance after a permitted kind
borrows nothing. A refusal is `policy/node_denied` (or `node_needs_approval`,
which reports `NEEDS_APPROVAL` exactly as the edge half does), carrying the
rule's own `reason`, under the POLICY check the planner already replans against.

**What a document that says nothing about nodes means.** `node_policy()` is
faithful to `check_node`, which means a document with no node rules and
`default = "deny"` compiles to a policy that denies every kind. That is the
right answer for the API and the wrong reading of an operator's intent, so
`grapharc plan --policy` compiles the node half only when the document declares
at least one `node` rule: saying nothing about nodes is not the same statement
as denying all of them, and the registry — an allowlist with no wildcard — is
still the gate in that case. `node_policy=None` on the checker means exactly
that, and is what every existing caller keeps.

Both halves now travel together as `GatePolicy` (`resolve_edge_policy` becomes
`resolve_policy`; `compile_policy` is the single place a document becomes
admission's objects, so `--policy`, a cached generated policy and a freshly
generated one cannot be read three different ways). The banner counts both:
`(tenant 'default', 1 edge rule(s), 2 node rule(s))`.

The repro above now refuses `deploy` with the operator's reason and replans
around it; `example.toml`'s node rules are enforced, and a test drives the
shipped document through the gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Shashankss1205
Shashankss1205 merged commit f35b722 into main Aug 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

policy: resource = "node" rules are silently dropped — a policy denying a node kind still admits and runs it

1 participant