refactor(policies): domain-agnostic engine with Resolver/Binding protocols#264
Open
jackmusick wants to merge 11 commits into
Open
refactor(policies): domain-agnostic engine with Resolver/Binding protocols#264jackmusick wants to merge 11 commits into
jackmusick wants to merge 11 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def resolve_reference(self, path: str) -> ColumnElement[Any]: | ||
| """Map a dot-path into a SQLAlchemy column expression.""" | ||
| ... |
|
|
||
| def resolve(self, path: str, ctx: Any) -> Any: | ||
| """Resolve a dot-path against the domain ctx. Missing returns None.""" | ||
| ... |
Design doc for #170. Adopts the policy skeleton from table-policies (feat/table-access) with a deliberately reduced v1 operator set (and/or/not/eq/call/literal — six AST node types) and a small {file: ...} reference set (created_by, created_at, path, location). Dependent on: - feat/table-access (table policies engine, manifest, editor pattern) - #155 (unified file path resolution, merged) Includes shared-engine extraction as a precondition (api/shared/policies/ becomes domain-agnostic; per-domain bindings move to table_policies.py / file_policies.py). Functions (has_role) shared from day one. Explicit fallback positions called out for OSS sustainability: - reduced operator set is viable on its own - shared-engine refactor is valuable for tables alone - batch signed-URL endpoint stands alone if policies slip
Domain-agnostic seams for the policy engine. Resolver handles reference lookup at evaluate time; Binding handles it at compile time. Subsequent commits move table-specific code behind these protocols.
Move Expr/Policy/PolicyDocument and the AST validator out of src/models/contracts/policies.py and into shared/policies/ast.py. The contracts file becomes a shim that re-exports KNOWN_USER_FIELDS, Expr, and defines a tables-narrowed Policy + TablePolicies preserving the table action vocab for existing callers.
Reference resolution moves behind the Resolver protocol. The walker
no longer hardcodes the 'row' namespace — {user: ...} and {call: ...}
stay in the walker (domain-agnostic), and any other single-key dict
goes to the Resolver.
probe.py and subscription.py carry a temporary _RowResolverForEngine
stub to keep tests green until Task 6 lands the real RowResolver in
shared.table_policies. test_evaluate.py and test_round_trip.py use
the same pattern via _RowResolverForTest stubs.
compile_to_sql now takes a Binding that knows how to project {row: ...}
references onto SQLAlchemy columns. The walker no longer imports
Document or _COLUMN_MAPPED_ROW_FIELDS.
probe.py carries a temporary _TableBindingForEngine stub so
compile_read_filter keeps working until Task 6 moves it to
shared.table_policies. test_compile.py and test_round_trip.py use
the same pattern via _TableBindingForTest stubs.
…e_policies Table-specific code moves out of the engine: - RowResolver + TableBinding (Protocol implementations) - compile_read_filter (SQL-pushdown helper, table-specific) - make_seed_admin_bypass (table action vocab) probe.py drops compile_read_filter and make_seed_admin_bypass and takes a Resolver parameter on evaluate_action / is_subscribe_authorized. subscription.py takes a Resolver. All temporary stubs from Tasks 4-5 are removed. tests/unit/policies/ uses the real RowResolver/TableBinding. routers/tables.py and routers/websocket.py will fail to import until Tasks 7-8 land — that's expected and scoped to those follow-up tasks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move policy helper imports (TablePolicies, compile_read_filter, make_seed_admin_bypass, RowResolver) from shared.policies.probe and contracts/policies to shared.table_policies. Pass RowResolver() to every evaluate_action and is_subscribe_authorized call site (4 in tables.py, 2 in websocket.py). No behavior change. Fixes all top-level ImportError crashes that prevented the API from starting after Task 6 dropped the moved helpers from probe.py. Also fixes lazy imports in cli.py, manifest_import.py, and mcp_server/tools/tables.py that would have failed at call time. tests/e2e/platform/test_policies.py: 21 passed tests/unit/policies/: 136 passed
… looseness
- test_admin_bypass_seed_migration.py imports make_seed_admin_bypass +
TablePolicies from shared.table_policies (not shared.policies.probe)
and passes RowResolver() to evaluate_action.
- Close a regression introduced by Task 3: the AST validator's
domain-reference branch accepted ANY single-key dict, silently
validating things like {has_role: 'support'} (an invalid shorthand
for the canonical {call: 'has_role', args: ['support']}). Add
_KNOWN_NAMESPACES = {'row', 'file'} allowlist; unknown single-key
ops now error with a clear message listing valid operators and
namespaces. test_manifest_import.py::test_table_policies_rejects_has_role_shorthand
goes back to green.
Adds two tests: - test_engine_does_not_import_domain_code: walks every module in shared/policies/ and fails if any import reaches into src.models.orm, shared.table_policies, or shared.file_policies. - test_forbidden_import_scanner_catches_violations: self-check that the scanner actually flags forbidden imports (without this, a buggy scanner returning [] would silently let regressions slip).
The engine extraction plan is what this PR executes. The file policies plan is the follow-up that consumes the refactored engine.
Single-screen pickup point — current state, what PR A shipped, what PR B will ship, how to resume, known quirks, and a decision log.
jackmusick
force-pushed
the
170-file-policies
branch
from
ca2751a to
66bf81f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Precondition for file policies (#170). Refactors
api/shared/policies/into a domain-agnostic engine consumed viaResolverandBindingprotocols, with table-specific code moved into a newapi/shared/table_policies.py. Tables behavior is bit-for-bit unchanged.This PR is independently valuable per the file-policies design doc §10.2 (engine refactor pays for itself for tables alone).
What changed
shared/policies/resolver.py,shared/policies/binding.py) — Resolver handles{<namespace>: ...}reference lookup at evaluate time; Binding handles it at compile time.shared/policies/ast.py) —Expr,Policy,PolicyDocument, and the AST validator moved out ofsrc/models/contracts/policies.py. The contracts file is now a thin shim that re-exports + defines a tables-narrowedPolicyto preserve the table action vocab (read/create/update/delete).shared/policies/evaluate.py) —evaluate(expr, ctx, user, resolver). No hardcoded"row"namespace; the walker dispatches onresolver.namespace.shared/policies/compile.py) —compile_to_sql(expr, user, binding). NoDocumentimport in the engine.shared/table_policies.py) — new module owningRowResolver,TableBinding,compile_read_filter,make_seed_admin_bypass, andTablePolicies = PolicyDocument. The only place that importssrc.models.orm.tables.Documentfrom the policy stack.{row: ...}and{file: ...}references opaquely; rejects unknown single-key dicts with a clear error (closes a regression where{has_role: "support"}was silently treated as a reference).routers/tables.py,routers/websocket.py,routers/cli.py,services/manifest_import.py,services/mcp_server/tools/tables.py,routers/export_import.py, andtests/unit/test_admin_bypass_seed_migration.pyuse the new import paths and passRowResolver()where required.tests/unit/policies/test_engine_protocols.py::test_engine_does_not_import_domain_codestatically asserts no module undershared/policies/importssrc.models.orm,shared.table_policies, orshared.file_policies. A self-check test confirms the scanner actually flags violations.Plan + spec
Test plan
No API endpoints or response models changed, so no client type regeneration needed.
🤖 Generated with Claude Code