Skip to content

feat(files): per-volume in-app policy enforcement - #197

Merged
atilafassina merged 19 commits into
mainfrom
files/service-principal-policies
Apr 22, 2026
Merged

feat(files): per-volume in-app policy enforcement#197
atilafassina merged 19 commits into
mainfrom
files/service-principal-policies

Conversation

@atilafassina

@atilafassinaatilafassina commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Per-volume policy system: FilePolicy — a function (action, resource, user) → boolean — gates every file operation before the Databricks API call. Ships with built-in helpers (policy.publicRead(), policy.allowAll(), policy.denyAll()) and combinators (all, any, not) for composition.
  • Service-principal execution model: HTTP routes now always execute as the service principal. The policy layer becomes the app-level gate — user identity is extracted from x-forwarded-user and passed to the policy function.
  • Safe defaults: Volumes without an explicit policy default to publicRead() (read-only). A startup warning encourages setting an explicit policy.

Motivation

Previously the files plugin relied entirely on OBO token forwarding and Unity Catalog grants. This meant:

  1. No app-level way to restrict per-user operations (e.g. allow reads but deny uploads).
  2. The SP's credentials were always used on HTTP routes regardless of UC grants.

Policies close this gap with a composable, per-volume authorization layer evaluated before any API call.

Key changes

AreaDetails
policy.ts (new)FileAction, FileResource, FilePolicyUser types · PolicyDeniedError · policy namespace with publicRead(), allowAll(), denyAll(), all(), any(), not()
plugin.tsPolicy enforcement wired into every route handler and programmatic API · Removed isInUserContext/OBO gating · Added _enforcePolicy(), _checkPolicy(), _extractUser() · _executeOrThrow() for proper 4xx propagation
types.tspolicy?: FilePolicy on VolumeConfig · Updated JSDoc to reflect SP-first execution model
index.tsRe-exports policy from top-level barrel
dev-playgroundUpdated to use policy.allowAll() on the default volume
docsFull plugin docs (files.md) with permission model, built-in policies, combinators, custom policies · Generated API reference for policy namespace

Test coverage

  • Policy combinators (policy.test.ts): all helpers, async policies, PolicyDeniedError, edge cases
  • Plugin enforcement (plugin.test.ts): route-level 401/403 responses, SDK API with user/SP identity, upload size in resource, cache invalidation under SP model
  • Integration tests updated for the policy-first model

Test plan

  • Unit tests for policy combinators and helpers
  • Unit tests for plugin policy enforcement across all route handlers
  • Existing plugin tests updated for new execution model
  • Linting and type checks pass
  • Manual verification against a Databricks workspace via pnpm dev

This pull request was AI-assisted by Isaac.

@atilafassinaatilafassina self-assigned this Mar 26, 2026
@atilafassina
atilafassina marked this pull request as ready for review March 26, 2026 13:56

@pkosiecpkosiec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Overall LGTM, pls see my small comments on the proposal and here - they might affect some methods naming etc.

Disclaimer: unfortunately I wasn't able to test it, I'll run it next week 👍 Unless you could record a short, raw demo video showcasing how the policies work ?

Also, I think it would be worth if we had approval for at least the RFC from Fabian / Mario who were much more involved in the files discussions. Thanks!

Comment threadpackages/appkit/src/plugins/files/policy.ts
Comment threadpackages/appkit/src/plugins/files/types.ts
CopilotAI review requested due to automatic review settings April 2, 2026 15:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an in-app, per-volume authorization layer to the Files plugin via composable FilePolicy functions, and updates the plugin so HTTP routes execute using the service principal while enforcing access decisions through policies (user identity taken from x-forwarded-user).

Changes:

  • Introduces FilePolicy types/helpers (policy.* combinators, PolicyDeniedError, READ/WRITE action sets) and wires policy checks into HTTP handlers and the programmatic Volume API.
  • Switches route execution to service-principal-first and enforces user identity presence (401) + policy decisions (403) before upstream Databricks calls.
  • Updates tests and documentation to reflect the new policy model and default publicRead() behavior.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 12 comments.

Show a summary per file
FileDescription
packages/appkit/src/plugins/files/policy.tsAdds policy types, helpers, combinators, and PolicyDeniedError.
packages/appkit/src/plugins/files/plugin.tsEnforces policies on routes and SDK API; defaults volumes to publicRead(); removes user-context gating.
packages/appkit/src/plugins/files/types.tsAdds policy?: FilePolicy to VolumeConfig and updates public API docs.
packages/appkit/src/plugins/files/index.tsRe-exports policy symbols from the files plugin barrel.
packages/appkit/src/index.tsExposes policy at top-level @databricks/appkit.
packages/appkit/src/context/*Removes isInUserContext and introduces getCurrentUserId usage for caching/identity.
packages/appkit/src/plugins/files/tests/policy.test.tsUnit tests for policy helpers/combinators.
packages/appkit/src/plugins/files/tests/plugin.test.tsAdds policy enforcement tests and updates behavior expectations (SP execution).
packages/appkit/src/plugins/files/tests/plugin.integration.test.tsUpdates integration tests for SP execution + default publicRead() denying writes.
apps/dev-playground/server/index.tsConfigures dev-playground volume with policy.allowAll().
docs/docs/plugins/files.mdDocuments the new policy model and enforcement semantics.
docs/docs/api/appkit/Variable.policy.mdAdds API reference page for policy.
docs/docs/api/appkit/index.mdAdds policy to API index.
docs/docs/api/appkit/typedoc-sidebar.tsAdds policy to Typedoc sidebar.
docs/static/appkit-ui/styles.gen.cssUpdates selection styling using color-mix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/docs/plugins/files.md Outdated
Comment threaddocs/docs/api/appkit/Variable.policy.md Outdated
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@databricksdatabricks deleted a comment from CopilotAIApr 2, 2026
@atilafassinaatilafassina changed the title feat(files): In-App Policy supportfeat(files): per-volume in-app policy enforcementApr 2, 2026
@atilafassina
atilafassinaforce-pushed the files/service-principal-policies branch from e43da85 to edabf4bCompareApril 2, 2026 19:27
Comment threadpackages/appkit/src/context/execution-context.ts
Comment threadapps/dev-playground/server/index.ts Outdated
Comment threadpackages/appkit/src/context/user-context.ts
Comment threadpackages/appkit/src/plugins/files/plugin.ts Outdated
Comment threaddocs/docs/plugins/files.md Outdated
@atilafassina
atilafassinaforce-pushed the files/service-principal-policies branch from b0faf25 to 6ef1adeCompareApril 13, 2026 08:33
Comment threadpackages/appkit/src/context/user-context.ts Outdated
Comment threadpackages/appkit/src/plugins/files/index.ts Outdated
Comment threadpackages/appkit/src/plugins/files/plugin.ts
Comment threadpackages/appkit/src/plugins/files/plugin.ts Outdated
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Add upload denyAll denial test for HTTP handler (closes coverage gap
on all 10 route handlers) and asUser() write-path tests for delete
and upload to verify policy enforcement on the programmatic API.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
@atilafassina
atilafassinaforce-pushed the files/service-principal-policies branch from 2600554 to 02225c7CompareApril 16, 2026 09:27
Comment threadpackages/appkit/src/plugins/files/plugin.ts
atilafassinaand others added 2 commits April 21, 2026 11:59
Malformed Content-Length headers (e.g. "abc", "123abc", "-1") were
parsed via parseInt which silently accepts partial matches and
negative values. This could bypass size-gating policies. Now strictly
validates with /^\d+$/ and returns 400 for invalid values.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
@atilafassina
atilafassina merged commit f54dca5 into mainApr 22, 2026
7 checks passed
@atilafassina
atilafassina deleted the files/service-principal-policies branch April 22, 2026 14:56
pkosiec added a commit that referenced this pull request Apr 30, 2026
The function was removed on main in feat(files): per-volume in-app
policy enforcement (#197) since the files plugin no longer needed it.
The telemetry interceptor needs it to set execution.context span
attributes.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
pkosiec added a commit that referenced this pull request Apr 30, 2026
The function was removed on main in feat(files): per-volume in-app
policy enforcement (#197) since the files plugin no longer needed it.
The telemetry interceptor needs it to set execution.context span
attributes.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
pkosiec added a commit that referenced this pull request Apr 30, 2026
* feat: add execution context attributes to telemetry spans
Add `execution.context` and `caller.id` span attributes to the
telemetry interceptor, allowing traces to distinguish OBO (user)
from service principal code paths.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* fix: preserve OTel context across async generator boundary in executeStream
The TelemetryInterceptor spans were orphaned because OTel lost the
parent HTTP span context when crossing into the async generator.
Capture context.active() before the generator and restore it with
context.with() inside, so plugin.execute spans appear as children
of the HTTP request trace.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* feat: add execution.obo_dev_fallback span attribute for OBO dev mode
When asUser() is called in dev mode without x-forwarded-access-token,
the telemetry span now includes execution.obo_dev_fallback: true to
distinguish intended OBO calls from regular service principal calls.
Uses OTel context key + thin proxy pattern to carry the flag without
mutable state — scoped automatically per execution and concurrent-safe.
Also documents telemetry span attributes in execution-context.md.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* test: add coverage for asUser() dev fallback proxy and OTel context preservation
Add tests for previously uncovered behaviors:
- asUser() dev fallback Proxy wraps methods correctly and sets isDevOboFallback context
- EXCLUDED_FROM_PROXY methods bypass OBO fallback wrapping
- executeStream preserves parent OTel context across async generator boundary
- isDevOboFallback() returns false outside proxy context
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* fix: use getCurrentUserId() for caller.id span attribute
The caller.id attribute was using context.userKey which is a cache key,
not always the real user ID. The analytics plugin passes "global" for SP
queries, so traces showed caller.id: "global" instead of the actual
service principal ID. Now uses getCurrentUserId() which always returns
the real identity.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* docs: document telemetry span attributes and interceptor chain requirement
Add telemetry span attributes table to execution-context.md and note
that execute()/executeStream() is required for automatic instrumentation.
Update custom-plugins.md to link telemetry attributes from the execution
interceptors bullet.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* feat: add db.user to lakebase spans and clarify arrow-result route
Add db.user attribute to lakebase.query telemetry spans so traces show
which PostgreSQL role executed the query. Also add a comment clarifying
that the arrow-result route intentionally bypasses the interceptor chain
(it's a data download, not a query execution).
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* fix: re-add isInUserContext() removed by files policy refactor
The function was removed on main in feat(files): per-volume in-app
policy enforcement (#197) since the files plugin no longer needed it.
The telemetry interceptor needs it to set execution.context span
attributes.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
---------
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
pkosiec added a commit that referenced this pull request May 21, 2026
* feat: add execution context attributes to telemetry spans
Add `execution.context` and `caller.id` span attributes to the
telemetry interceptor, allowing traces to distinguish OBO (user)
from service principal code paths.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* fix: preserve OTel context across async generator boundary in executeStream
The TelemetryInterceptor spans were orphaned because OTel lost the
parent HTTP span context when crossing into the async generator.
Capture context.active() before the generator and restore it with
context.with() inside, so plugin.execute spans appear as children
of the HTTP request trace.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* feat: add execution.obo_dev_fallback span attribute for OBO dev mode
When asUser() is called in dev mode without x-forwarded-access-token,
the telemetry span now includes execution.obo_dev_fallback: true to
distinguish intended OBO calls from regular service principal calls.
Uses OTel context key + thin proxy pattern to carry the flag without
mutable state — scoped automatically per execution and concurrent-safe.
Also documents telemetry span attributes in execution-context.md.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* test: add coverage for asUser() dev fallback proxy and OTel context preservation
Add tests for previously uncovered behaviors:
- asUser() dev fallback Proxy wraps methods correctly and sets isDevOboFallback context
- EXCLUDED_FROM_PROXY methods bypass OBO fallback wrapping
- executeStream preserves parent OTel context across async generator boundary
- isDevOboFallback() returns false outside proxy context
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* fix: use getCurrentUserId() for caller.id span attribute
The caller.id attribute was using context.userKey which is a cache key,
not always the real user ID. The analytics plugin passes "global" for SP
queries, so traces showed caller.id: "global" instead of the actual
service principal ID. Now uses getCurrentUserId() which always returns
the real identity.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* docs: document telemetry span attributes and interceptor chain requirement
Add telemetry span attributes table to execution-context.md and note
that execute()/executeStream() is required for automatic instrumentation.
Update custom-plugins.md to link telemetry attributes from the execution
interceptors bullet.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* feat: add db.user to lakebase spans and clarify arrow-result route
Add db.user attribute to lakebase.query telemetry spans so traces show
which PostgreSQL role executed the query. Also add a comment clarifying
that the arrow-result route intentionally bypasses the interceptor chain
(it's a data download, not a query execution).
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
* fix: re-add isInUserContext() removed by files policy refactor
The function was removed on main in feat(files): per-volume in-app
policy enforcement (#197) since the files plugin no longer needed it.
The telemetry interceptor needs it to set execution.context span
attributes.
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
---------
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
atilafassina added a commit to databricks/databricks-agent-skills that referenced this pull request May 22, 2026
Update the Files skill reference for AppKit v0.21.0+ per-volume
policy enforcement (PR databricks/appkit#197):
- Add Permission Model section explaining the three layers
(UC grants, execution identity, file policies)
- Add Access Policies section with built-ins, combinators,
custom policies, and policy-input types
- Fix HTTP Routes intro: routes always run as service principal,
user identity comes from x-forwarded-user and is passed to
the policy (not asUser(req))
- Reframe Server-Side API: asUser(req) switches policy identity,
not UC credentials
- Refresh troubleshooting with policy-related errors and the
default-publicRead() write-denial gotcha
Co-authored-by: Isaac
atilafassina added a commit to databricks/databricks-agent-skills that referenced this pull request May 22, 2026
Update the Files skill reference for AppKit v0.21.0+ per-volume
policy enforcement (PR databricks/appkit#197):
- Add Permission Model section explaining the three layers
(UC grants, execution identity, file policies)
- Add Access Policies section with built-ins, combinators,
custom policies, and policy-input types
- Fix HTTP Routes intro: routes always run as service principal,
user identity comes from x-forwarded-user and is passed to
the policy (not asUser(req))
- Reframe Server-Side API: asUser(req) switches policy identity,
not UC credentials
- Refresh troubleshooting with policy-related errors and the
default-publicRead() write-denial gotcha
Co-authored-by: Isaac
atilafassina added a commit to databricks/databricks-agent-skills that referenced this pull request Jun 3, 2026
## Summary
- Updates the Files skill reference for AppKit v0.21.0+ per-volume
policy enforcement
([databricks/appkit#197](databricks/appkit#197))
- Adds **Permission Model** + **Access Policies** sections covering
built-ins (`publicRead`, `allowAll`, `denyAll`), combinators (`all`,
`any`, `not`), custom `FilePolicy` shape, exported types, and
`PolicyDeniedError`
- Fixes a factually wrong HTTP Routes line — routes always run as the
service principal; user identity comes from `x-forwarded-user` and is
passed to the policy (not via `asUser(req)`)
- Reframes the Server-Side API section so `asUser(req)` is described as
switching the **policy identity**, not the UC credentials
- Refreshes the troubleshooting table with policy-related rows,
including the `publicRead()` default-write-denial gotcha
## Test plan
- [x] `python3 scripts/skills.py validate` passes
- [ ] Skim rendered Markdown for the new Permission Model / Access
Policies sections
- [ ] Confirm policy snippets compile against `@databricks/appkit`
v0.31.0 types
This pull request and its description were written by Isaac.
Sign up for freeto 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.

4 participants

@atilafassina@pkosiec@MarioCadenas