Skip to content

fix: Scope offline server permission checks to the requested project - #6803

Open
hiyufan wants to merge 1 commit into
feast-dev:masterfrom
hiyufan:fix/offline-server-project-scoped-permissions
Open

fix: Scope offline server permission checks to the requested project#6803
hiyufan wants to merge 1 commit into
feast-dev:masterfrom
hiyufan:fix/offline-server-project-scoped-permissions

Conversation

@hiyufan

@hiyufan hiyufan commented Sep 1, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

SecurityManager.permissions is loaded per project, and every request-facing path is
supposed to bind it to the project the request names before any permission check runs.
The registry server does this — api/registry/rest/rest_utils.py,
permissions/server/rest.py, permissions/server/grpc.py and registry_server.py all
call set_current_project(...) and reset it afterwards.

The Arrow Flight offline server does not. Neither of its two dispatchers —
OfflineServer.do_get and OfflineServer._call_api — touches the current project, so
every assert_permissions call in the handlers below them is evaluated against the
Permission list of whatever project the SecurityManager was constructed from, which
is the project in the server's own feature_store.yaml.

An offline server that serves more than one project therefore enforces its home
project's policy on every request. A role granted only in that project reaches the other
projects' data sources, feature views and saved datasets, and the policies those projects
defined for themselves are never loaded. get_historical_features already asserts that
project is mandatory in its command and uses it to resolve feature views, so the
per-request project was available at the dispatcher the whole time — it just never
reached the permission layer.

The fix

Bind the SecurityManager to command["project"] around both dispatchers and reset it
in a finally, following the interceptor in permissions/server/grpc.py:

project_token = self.store.set_current_project(command.get("project"))
try:
    ...
finally:
    self.store.reset_current_project(project_token)

A command that carries no project passes None. SecurityManager.permissions already
falls back from None to the server's own project, so those paths — offline_write_batch
and write_logged_features, which read self.store.config.project — behave exactly as
before.

Which issue(s) this PR fixes:

Relates to #6784 (advisory GHSA-5px7-7gwg-6g93).

That issue is written against the registry server, and the registry paths already scope
the lookup on master. This is the same defect on the Arrow Flight offline server, which
was not covered. I have left it as "relates to" rather than "fixes" so you can decide
whether it closes the issue or belongs as a follow-up.

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests

Four tests in sdk/python/tests/unit/test_offline_server.py:

  • test_do_get_scopes_permissions_to_the_requested_project
  • test_call_api_scopes_permissions_to_the_requested_project
  • test_call_api_resets_the_project_when_the_handler_raises — a failed request must not
    leave its project bound for the next one
  • test_call_api_without_a_project_leaves_the_lookup_unchanged — pins the None
    fallback, so the paths that do not send a project keep their current behaviour

All four fail on the base revision and pass with the change:

# base revision, new tests only
4 failed

# this branch
$ pytest sdk/python/tests/unit/test_offline_server.py
14 passed

$ pytest sdk/python/tests/unit/test_offline_server.py sdk/python/tests/unit/permissions/
343 passed

pre-commit passes on both changed files.

Misc

The scope here is deliberately narrow: it moves the offline server onto the same
per-request project binding the registry server already uses, and changes nothing about
how permissions themselves are defined or evaluated. If you would rather see the binding
lifted into a shared Flight middleware — the way permissions/server/grpc.py does it for
gRPC, instead of at each dispatcher — I am happy to rework it that way.


This change was developed with AI assistance. The test runs and the base-revision
comparison reported above were executed locally against this branch as submitted.

The registry server's REST and gRPC paths bind the SecurityManager to the
project each request names, so its permission list is loaded for that project.
The Arrow Flight offline server never did: both of its dispatchers ran the
handlers, and therefore `assert_permissions`, with whatever project the
SecurityManager was constructed from — the project in the server's own
`feature_store.yaml`.

An offline server that serves more than one project consequently enforced its
home project's `Permission` list against every request, so a role granted only
in that project reached the other projects' data sources and feature views,
while the policies those projects defined for themselves were never consulted.
`get_historical_features` already requires a `project` in its command, so the
per-request project was available all along.

Bind the SecurityManager to `command["project"]` around both dispatchers and
reset it afterwards, following the interceptor in permissions/server/grpc.py. A
command that carries no project passes `None`, which the SecurityManager falls
back from to the server's own project, leaving those paths unchanged.

Fixes feast-dev#6784

Signed-off-by: Chen Yufan <yufan_ai@outlook.com>
@hiyufan
hiyufan requested a review from a team as a code owner September 1, 2026 05:06
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.

1 participant