Skip to content

planner: the admission gate authorises a kind, not its arguments #10

Description

@Shashankss1205

Summary

Two related gaps in the admission gate, both documented and both deliberate:

Arguments are not authorised. No rule reaches ProposedNode.args, so a proposal carrying args={"path": "/etc/passwd"} is admitted on the strength of its kind alone. Materializer drops args by default (forward_args=False); turning that on hands a model's unchecked dictionary to your factory, and gating it becomes the factory's job.

parent_depth is the caller's word. The checker cannot see how deep the run actually is, so a caller that always passes 0 has no recursion limit beyond the nesting visible inside a single proposal.

Why this matters

The admission gate is the part of this project with no prior art to copy, and it is the reason the rest exists: a planner proposes, a deterministic checker admits, and only then does anything execute. Its five checks are strong — kind in the registry, edge permitted between kinds, worst case within the remaining budget, depth within limit, acyclic — and a decision keyed on kind rather than instance name means renaming cannot launder a denied capability. Closing the argument gap is what would let graph engineering claim that an admitted proposal is safe to run rather than merely well-shaped, which is the difference between a gate and a shape check.

Where in the code

  • grapharc/planner/admission.pyAdmissionChecker.check and the five checks; :775 _find_cycle; :271 the tier-ordering note
  • grapharc/planner/proposal.pyProposedNode, Subgraph, _NAME (note extra="forbid", so a proposal cannot carry code)
  • grapharc/planner/materialize.pyMaterializer, forward_args, and the fingerprint match that binds materialisation to the authorisation
  • grapharc/planner/loop.py — where parent_depth is passed
  • grapharc/policy/engine.pycheck_node, if argument rules belong in the document

What to change

Two separable pieces; either is a valid PR.

Argument authorisation. The design question is where the schema comes from.

  • A NodeSpec could declare an args schema (a Pydantic model) that the checker validates a proposal's args against. This keeps the registry as the source of truth, which matches the existing rule that costs come from the registry, never the proposal.
  • Or argument rules live in the policy TOML, next to the node and edge rules.
  • Whichever you choose: a rejection must remain data. AdmissionResult.feedback() hands the per-check list with codes and remedies back to the planner as its next round's input, and the loop never retries an identical proposal. A new check must produce a reason code and a remedy in that same shape, and must appear in the admission trace event's failed-check list.
  • Nothing may run during a check: NodeSpec.factory is never called and the budget meter is read, not written. Validation must not violate that.
  • Decide what happens to forward_args=False. If args are now authorised, is forwarding them still off by default?

Real depth. Give the checker a trustworthy depth rather than a caller-supplied integer — most likely by threading it through RunContext or the loop's own state, so a caller cannot understate it. Then decide whether an understated depth is a rejection or an error.

How to verify

uv run pytest tests/test_admission.py tests/test_planner_loop.py -q
uv run pytest -q
uv run ruff check .

Follow the adversarial style already in tests/test_admission.py: renaming a denied kind does not evade the policy, nor does hiding the rename in a nested scope, and every failed check is reported rather than just the first. New tests should include a traversing path in args being refused, and a caller that lies about parent_depth gaining nothing.

Acceptance criteria

  • A proposal whose arguments violate a declared schema or rule is refused, with a reason code and remedy in feedback()
  • Nothing executes during a check — no factory call, no meter write
  • Every failed check is still reported, not just the first
  • The refusal appears on the admission trace event and does not inflate node-execution counts
  • Materialisation still binds to the authorisation by fingerprint
  • Depth cannot be understated by the caller (if you take that half)
  • The README paragraphs stating these two limits are updated
  • uv run pytest green, uv run ruff check . clean

Skill level — experience required

This is the security core of the project, and the existing tests are adversarial on purpose — they assume someone is trying to get a denied capability past the gate. You need to understand why every decision keys on kind and never on name, why a proposal cannot carry code, and why a rejection is data rather than a downgraded approval, before you change what is checked. Please propose your design in a comment first; a check that can be bypassed is worse than a documented gap, because the gap is at least honest.

Metadata

Metadata

Assignees

No one assigned

    Labels

    architectureChanges a subsystem boundary or a cross-cutting contractenhancementNew feature or requestexperience requiredDeep familiarity with the codebase or domain needed; not a starter taskhelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions