Skip to content

fix(prd-140): unblock + harden platform-tool authz gate (org-chart writes)#399

Merged
AutomatosAI merged 2 commits into
mainfrom
fix/prd-140-permission-crash
May 29, 2026
Merged

fix(prd-140): unblock + harden platform-tool authz gate (org-chart writes)#399
AutomatosAI merged 2 commits into
mainfrom
fix/prd-140-permission-crash

Conversation

@AutomatosAI
Copy link
Copy Markdown
Owner

@AutomatosAI AutomatosAI commented May 29, 2026

Summary

The PRD-140 hierarchy permission gate (can_actor_modify) — the single chokepoint for mutating platform_* tool actions — was crashing every gated platform write (e.g. Auto's platform_update_agent / org-chart edits) because its keyword-only workspace_id parameter had no default and the dispatcher never passed it. This PR fixes the crash and hardens the gate to a deny-by-default, workspace-bound contract, and closes an actor-spoofing path.

What was broken

What this PR does

  • Actor gate fails closed: anonymous (no actor id), unknown, cross-workspace, and inactive actors are denied. Absence of identity is no longer treated as trusted ambient authority.
  • Narrowed system bypass: requires is_system_agent=True AND a name on SYSTEM_BYPASS_ALLOWLIST (Auto, Auto CTO, onboarding agents, service actors). A stray is_system_agent flag alone is no longer a master key.
  • Workspace-bound everywhere: cross-tenant targets/owners are denied (IDOR closed) before any hierarchy reasoning.
  • Escalation discipline: escalation_target="auto" is set only on authority limits (out-of-subtree, unresolved owner, skill, broken hierarchy) so legitimate-but-unauthorized actors route to Auto for arbitration. Security failures get no escalation hint.
  • Actor-spoofing closed: exec_platform now server-mints actor identity from the trusted runtime agent_id only — it strips any LLM-supplied _agent_id/_agent_name before reapplying, removing an impersonation path that bypassed the gate entirely.

Auto retains org-chart authority via the allowlist bypass (it calls with a truthy runtime agent_id).

Test plan

  • pytest tests/security/test_hierarchy_permissions.py — 24/24 green (anonymous/unknown/cross-workspace/inactive deny, narrowed-allowlist bypass, subtree/owner scoping, cross-tenant target deny)
  • python scripts/check_hierarchy_gate.py — CI gate green (19 gated / 28 allow-list / 46 mutating)
  • Confirmed platform_executor.py:552 is the sole caller of can_actor_modify — no other call site crashes on the now-passed workspace_id
  • Post-deploy: verify Auto can run platform_update_agent (org-chart write) on the live URL

Summary by CodeRabbit

Release Notes

  • Security Improvements
    • Implemented strict, deny-by-default access control for workspace hierarchy operations
    • Restricted system agent bypass to require both system flag and allowlisted agent name
    • Added workspace-scoped permission validation to prevent unauthorized cross-tenant data access
    • Hardened platform action execution to prevent malicious or accidental agent impersonation through parameter manipulation

Review Change Stack

…atform writes

The merged PRD-140 helper required a keyword-only `workspace_id` (no default)
and exposed no `escalation_target`, but the sole caller
(`platform_executor`) invokes it without `workspace_id` and reads
`decision.escalation_target`. Every gated `platform_*` write/destructive
action (org-chart edits via `platform_update_agent`, agent updates, task/
playbook/tool mutations) therefore raised TypeError and failed closed —
surfacing to Auto as a "missing workspace authorization/context" error.

Rewrite the helper to the contract its caller and the 20 hierarchy tests
expect:
- signature `(db, *, actor_agent_id, target_type, target_id=None, change_type="update")`
- `PermissionDecision` regains `escalation_target` (set to "auto" on
  escalatable denials) while keeping every field bypass_audit consumes
- model: None actor -> allow (trusted platform flow); is_system_agent ->
  allow (Auto bypass); otherwise scope to the actor's reports_to_id subtree,
  resolving task/playbook owners via SAVEPOINT-guarded lookups so a missing
  column escalates instead of crashing the caller's transaction

20/20 hierarchy tests pass; CI hierarchy gate and bypass_audit unchanged.
…orm tools

The prior commit restored the gate's signature but left a weak contract.
Re-harden to deny-by-default and make every check workspace-bound:

- Actor gate fails closed: anonymous (no actor id), unknown, cross-workspace,
  and inactive actors are denied. Absence of identity is no longer treated as
  trusted ambient authority — trusted flows must pass an explicit seeded actor.
- System bypass is narrowed: requires is_system_agent=True AND a name on
  SYSTEM_BYPASS_ALLOWLIST (Auto, Auto CTO, onboarding agents, service actors).
  A stray is_system_agent flag alone is no longer a master key.
- Every scope check is workspace-bound; cross-tenant targets/owners denied
  (IDOR closed) before any hierarchy reasoning.
- escalation_target="auto" is set only on authority limits (out-of-subtree,
  unresolved owner, skill, broken hierarchy) so legitimate-but-unauthorized
  actors route to Auto. Security failures get no escalation hint.
- platform_executor passes workspace_id + source into the gate.
- exec_platform now server-mints actor identity from the trusted runtime
  agent_id only: strip any LLM-supplied _agent_id/_agent_name before reapply,
  closing an impersonation path that bypassed the gate entirely.

Auto retains org-chart authority via the allowlist bypass. 24 gate tests +
CI hierarchy gate green.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 43260355-08b4-4455-bb82-8be6eed3e061

📥 Commits

Reviewing files that changed from the base of the PR and between 6995e24 and 44f7344.

📒 Files selected for processing (4)
  • orchestrator/core/security/hierarchy_permissions.py
  • orchestrator/modules/tools/discovery/platform_executor.py
  • orchestrator/modules/tools/execution/exec_platform.py
  • orchestrator/tests/security/test_hierarchy_permissions.py

📝 Walkthrough

Walkthrough

This PR hardens the hierarchy permission system into a deny-by-default, workspace-scoped authorization chokepoint. can_actor_modify implements explicit fail-closed actor gating for anonymous/missing/cross-workspace/inactive actors, narrows system-agent bypass to allowlisted names, routes different target types through scoped evaluation helpers, and signals arbitration-eligible denies via escalation_target="auto". Agent identity injection is hardened against impersonation, and platform tool execution now provides workspace context for authorization.

Changes

Workspace-Scoped Hierarchy Authorization with Deny-by-Default Gate

Layer / File(s) Summary
Permission Model & Constants
orchestrator/core/security/hierarchy_permissions.py
PermissionDecision gains escalation_target: Optional[str] field to distinguish denial types. Module constants updated: TARGET_* string values remain as public API, SYSTEM_BYPASS_ALLOWLIST expanded to include platform-seeded agent names (Auto CTO, VOYAGER, BLUEPRINT, SCRIBE, FORGE). Documented subtree depth/cycle handling expectations.
Authorization Engine & Scope Evaluation
orchestrator/core/security/hierarchy_permissions.py
can_actor_modify refactored as workspace-scoped permission chokepoint implementing fail-closed actor gate (anonymous/missing/cross-workspace/inactive deny without escalation), narrowed system bypass (requires both is_system_agent and allowlisted name), and per-target-type routing via three scope helpers: _scope_to_agent (validates agent targets, allows self-mutation), _scope_via_owner (resolves task/playbook owners, escalates on unresolved), _by_subtree (arbiter allowing on membership, denying with escalation_target="auto" for broken hierarchy/cycle/depth).
SQL & Decision Support Infrastructure
orchestrator/core/security/hierarchy_permissions.py
Lightweight raw SQL helpers replace full ORM model loads: _agent_row (fetch agent by ID), _reports_to_id (resolve reporting chain), _owner_id (query with savepoint error handling for unresolved cases), _in_subtree (walks chain with cycle detection and depth cap, returns None for broken hierarchy). Utility functions for type coercion (_as_int, _sid, _ws), result construction (_allow), and updated _deny with conditional escalation_target setting.
Agent Identity Injection Security Hardening
orchestrator/modules/tools/execution/exec_platform.py
execute_platform_action sanitizes incoming parameters by stripping any caller/LLM-supplied _agent_id or _agent_name keys before injecting trusted actor context. Injects _agent_id from trusted agent_id argument and conditionally resolves _agent_name from Agent model for the given workspace_id, logging and continuing on lookup failures.
Platform Tool Authorization Wiring
orchestrator/modules/tools/discovery/platform_executor.py
PlatformActionExecutor.execute() now passes workspace_id=self.workspace_id and source="platform_tool" to can_actor_modify alongside existing target identification and change_type arguments.
Test Contract & Schema Fixtures
orchestrator/tests/security/test_hierarchy_permissions.py
Test PRD documents deny-by-default security contract (fail-closed actor gate, narrowed system bypass, per-target-type scoping, escalation routing). SQLite schema updated with workspace_id and status columns on agents table for cross-tenant and inactive-actor test cases. Test constants WS/WS2 define two workspaces.
Authorization & Scoping Test Coverage
orchestrator/tests/security/test_hierarchy_permissions.py
Comprehensive tests validate actor gating (anonymous/missing/cross-workspace/inactive denials with no escalation), restricted system bypass (allowlist-name requirement, non-allowlisted system-flag denial), subtree rules (self-edit allowed, cross-workspace target denied, missing assignee/owner escalation to "auto"), and unknown target_type denial.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • AutomatosAI/automatos-ai#295: Prior PR implementing the initial can_actor_modify contract and hierarchy authorization semantics that this PR hardened and refactored into a single deny-by-default chokepoint.

Poem

🐰 A hierarchy in order, with gates that stand so tall,
Denying by default, now nobody crawls.
Through subtrees and scopes, the system agent strode,
With names on the allowlist—the secure, tested road.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: fixing and hardening the platform-tool authorization gate for organization chart writes (PRD-140), which is the core objective of the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/prd-140-permission-crash

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@AutomatosAI AutomatosAI merged commit 21f6db7 into main May 29, 2026
4 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.

2 participants