fix(auth): guard FGAC checker build in SessionHL::Pull on valid license by andrejtonev · Pull Request #4322 · memgraph/memgraph · GitHub
Skip to content

fix(auth): guard FGAC checker build in SessionHL::Pull on valid license#4322

Draft
andrejtonev wants to merge 1 commit into
masterfrom
fix/fgac-null-checker-license-dropped
Draft

fix(auth): guard FGAC checker build in SessionHL::Pull on valid license#4322
andrejtonev wants to merge 1 commit into
masterfrom
fix/fgac-null-checker-license-dropped

Conversation

@andrejtonev

Copy link
Copy Markdown
Contributor

SessionHL::Pull built a fine-grained auth checker whenever an authenticated user was present, but -- unlike the three call sites in interpreter.cpp -- did not first check that the enterprise license is valid. AuthChecker::GetFineGrainedAuthChecker returns a null unique_ptr the moment the license is invalid, so on an enterprise binary running without a valid license the following auth_checker->NeedsFineGrainedAuthChecker() dereferenced null and crashed the server (the DMG_ASSERT guarding it is compiled out in release builds).

SessionHL::Pull built a fine-grained auth checker whenever an authenticated
user was present, but -- unlike the three call sites in interpreter.cpp -- did
not first check that the enterprise license is valid.
AuthChecker::GetFineGrainedAuthChecker returns a null unique_ptr the moment the
license is invalid, so on an enterprise binary running without a valid license
the following `auth_checker->NeedsFineGrainedAuthChecker()` dereferenced null
and crashed the server (the DMG_ASSERT guarding it is compiled out in release
builds).

Restore the invariant the DMG_ASSERT documents by gating the block on
license::global_license_checker.IsEnterpriseValidFast(), matching the other
call sites. When the license is invalid, fine-grained filtering is simply not
applied (community-legal) and the query returns cleanly.

The crash reproduces whenever an enterprise binary runs unlicensed and any
authenticated user (QueryUserOrRole is truthy iff a username is set -- a role is
not required) runs a data query. QA hit it by creating a role under a valid
license and then dropping the license; that role and license drop are
incidental to the crash.

Regression test: fine_grained_access/unlicensed_user_query_crash_tests.py.
It cannot use a runner-managed `cluster:` instance because CI supplies the
license via env (MEMGRAPH_ENTERPRISE_LICENSE / MEMGRAPH_ORGANIZATION_NAME) and
the license resolver keeps the valid ENV candidate even after `SET DATABASE
SETTING 'enterprise.license' TO ''`, so the license can't be dropped at runtime.
Instead the test self-manages its instance via interactive_mg_runner and pops
the license env vars before start (Popen inherits the mutated os.environ), so
Memgraph boots unlicensed. It then creates a user, runs a data query as that
user, and asserts the query returns cleanly and the server stays alive.

Claude-Session: https://claude.ai/code/session_01S7bHLYsqWYDtsuR9oK3K37
@andrejtonev andrejtonev added this to the mg-v3.12.0 milestone Jul 1, 2026
@andrejtonev andrejtonev requested a review from colinbarry July 1, 2026 11:41
@andrejtonev andrejtonev self-assigned this Jul 1, 2026
@andrejtonev andrejtonev added bug bug Docs - changelog only Docs - changelog only CI -build=coverage -test=core Run coverage build and core tests on push CI -build=debug -test=core Run debug build and core tests on push CI -build=release -test=e2e Run release build and e2e tests on push CI -build=coverage -test=clang_tidy labels Jul 1, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jul 1, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug bug CI -build=coverage -test=clang_tidy CI -build=coverage -test=core Run coverage build and core tests on push CI -build=debug -test=core Run debug build and core tests on push CI -build=release -test=e2e Run release build and e2e tests on push Docs - changelog only Docs - changelog only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant