Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
746fe27
fix(subscriptions): auto-disable when the owner left or lost access
a-lider Oct 26, 2026
8b68ec9
fix(alerts): shorten owner disable reasons and clarify the owner check
a-lider Oct 26, 2026
b5c0aa5
fix(subscriptions): send the reason in the delivery, then disable
a-lider Oct 26, 2026
a02ae6b
fix(alerts): record owner disables as invalid_configuration
a-lider Oct 26, 2026
68bd3ef
fix(alerts): move the owner check into the shared insight evaluator
a-lider Oct 26, 2026
552d975
chore(subscriptions): make the test user a member of the temporal tea…
a-lider Oct 28, 2026
7f8ddfb
fix(subscriptions): count a deleted owner and lost project access as …
a-lider Oct 28, 2026
895d515
fix(subscriptions): deliver a partly denied dashboard, disable even w…
a-lider Oct 29, 2026
620a1bb
fix(subscriptions): stop AI subscriptions whose owner can no longer r…
a-lider Oct 29, 2026
4517b08
Merge remote-tracking branch 'origin/master' into alex/auto-disable-r…
a-lider Oct 29, 2026
18bfe21
fix(subscriptions): stop when the owner can no longer view the dashbo…
a-lider Oct 29, 2026
47cd10f
refactor(subscriptions): one owner check for background queries
a-lider Oct 29, 2026
b56f00e
fix(subscriptions): a member who lost the project lost access, not left
a-lider Oct 29, 2026
150add7
chore(exports): read dashboard_id in the owner test
a-lider Oct 29, 2026
9008fb5
fix(subscriptions): check project access through the facade
a-lider Oct 29, 2026
3f9c518
refactor(subscriptions): raise inside the object access condition
a-lider Oct 29, 2026
42c2a39
fix(subscriptions): read has_project_access as a property, keep trans…
a-lider Oct 29, 2026
d365dab
refactor(subscriptions): drop the disable closure in deliver_subscrip…
a-lider Oct 29, 2026
d1b0665
chore(subscriptions): stub the owner check in database-free delivery …
a-lider Oct 29, 2026
ac7b51b
fix(subscriptions): keep a transient send failure from disabling the …
a-lider Nov 2, 2026
bb87e9d
fix(alerts): check the owner on the per-series evaluation path too
a-lider Nov 2, 2026
3c17c0d
fix(subscriptions): treat an ownerless subscription export as a delet…
a-lider Nov 2, 2026
29e3273
Merge remote-tracking branch 'origin/master' into alex/auto-disable-r…
a-lider Nov 2, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/debugging-table-access-denied/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ Hints:
- On an HTTP request this is an expected 4xx, never captured. An error tracking issue means it was raised in a background context — figure out which from the event's query tags (`team_id`, `user_id`, `product`, `celery_task_id`, `temporal.*`).
- A `system.*` name is a system-table decision; a bare name is a warehouse table/view denial.
- No `user_id` on the event usually means a userless `Database.create_for` — it fails closed, and `bypass_warehouse_access_control` does not cover system tables. That's a bug in the calling code.
- Background jobs run as the resource's `created_by`; a creator who left the org is the known benign cause (cache warming, alerts, and exports already suppress capture for it via `creator_access_revoked`).
- Background jobs run as the resource's `created_by`. A creator who left the org is the known benign cause: cache warming suppresses capture for it via `creator_access_revoked`, exports refuse to run with `CreatorLeftOrganizationError`, and subscriptions and alerts auto-disable (look for the `alert auto disabled` event or the subscription's last delivery).
- Otherwise it's a real access rule (RBAC or entitlement) — see the readme for how a table's access level resolves, and check the `AccessControl` rows for the table's `access_scope`.
39 changes: 39 additions & 0 deletions posthog/query_creator_access.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
from typing import Any, Optional

from django.db.models import Model

from posthog.models.organization import OrganizationMembership
from posthog.models.team import Team
from posthog.models.user import User
from posthog.ph_client import ph_scoped_capture

from products.access_control.backend.facade.user_access_control import (
UserAccessControl,
UserAccessControlError,
model_to_resource,
)


class CreatorLeftOrganizationError(Exception):
"""A background query refused to run because its owner is no longer in the organization."""

def __init__(self) -> None:
super().__init__("The owner is no longer in the organization.")


def creator_access_revoked(user: Optional[User], team: Team) -> bool:
"""True when `user` no longer has access to `team`: they were deleted (`user is None`),
Expand All @@ -17,6 +32,30 @@
return not OrganizationMembership.objects.filter(organization_id=team.organization_id, user=user).exists()


def assert_creator_can_run(user: Optional[User], team: Team, *objects: Model | None) -> None:
"""Raise when `user` can no longer run a background query for `team` as its owner.

`CreatorLeftOrganizationError` when they were deleted (`user is None`), deactivated, or left the
team's org. `UserAccessControlError` when they are still in the org but cannot view the project
or one of `objects`. Background jobs load their dashboard and insight directly, so nothing else
checks object access for them. Organization admins pass, and a `None` object is skipped.
"""
if user is None or creator_access_revoked(user, team):
raise CreatorLeftOrganizationError()
user_access_control = UserAccessControl(user=user, team=team)
# Object rules do not consult the project, so a member shut out of a private project is checked first.
if not user_access_control.has_project_access():

Check failure on line 47 in posthog/query_creator_access.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

"bool" not callable
Comment thread
a-lider marked this conversation as resolved.
Resolved
Comment thread
a-lider marked this conversation as resolved.
Resolved
raise UserAccessControlError("project", "viewer", str(team.pk))
for obj in objects:
if obj is None:
continue
resource = model_to_resource(obj)
# A model with no scope has no access rules, so there is nothing to deny.
if resource is None or user_access_control.check_access_level_for_object(obj, "viewer"):
continue
raise UserAccessControlError(resource, "viewer", str(obj.pk))


def report_creator_access_revoked(
*,
user: Optional[User],
Expand Down
4 changes: 4 additions & 0 deletions posthog/tasks/exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from prometheus_client import Counter, Histogram

from posthog.event_usage import EventSource, groups
from posthog.query_creator_access import assert_creator_can_run
from posthog.settings import HOGQL_INCREASED_MAX_EXECUTION_TIME
from posthog.tasks.utils import CeleryQueue

Expand Down Expand Up @@ -121,6 +122,9 @@ def export_asset_direct(
export_source = source or EventSource.EXPORT

try:
# Some exports are made with no owner on purpose (logs, activity logs), so only an owned one can lose its owner.
if exported_asset.created_by is not None:
Comment thread
a-lider marked this conversation as resolved.
Resolved
assert_creator_can_run(exported_asset.created_by, team, exported_asset.dashboard, exported_asset.insight)
assert_export_authorization(exported_asset)
if exported_asset.export_format in (ExportedAsset.ExportFormat.CSV, ExportedAsset.ExportFormat.XLSX):
csv_exporter.export_tabular(exported_asset, limit=limit, source=export_source)
Expand Down
45 changes: 45 additions & 0 deletions posthog/tasks/test/test_exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@

from posthog.hogql.errors import QueryError

from posthog.constants import AvailableFeature
from posthog.exceptions import ClickHouseAtCapacity
from posthog.models import OrganizationMembership, User
from posthog.models.personal_api_key import PersonalAPIKey
from posthog.models.utils import generate_random_token_personal, hash_key_value
from posthog.tasks import exporter

from products.access_control.backend.models.access_control import AccessControl
from products.dashboards.backend.models.dashboard import Dashboard
from products.exports.backend.models.exported_asset import DATASET_EXPORT_KIND, ExportedAsset
from products.exports.backend.tasks.failure_handler import (
Expand Down Expand Up @@ -122,6 +125,48 @@ def test_export_stores_exception_type_on_failure(self, mock_export: MagicMock, m
assert asset.exception_type == "QueryError"
assert asset.failure_type == FAILURE_TYPE_USER

@parameterized.expand(
[
("owner_left_the_organization", False, "CreatorLeftOrganizationError"),
("owner_cannot_view_the_dashboard", True, "UserAccessControlError"),
]
)
@patch("products.exports.backend.tasks.image_exporter.export_image")
def test_export_does_not_run_when_its_owner_cannot(
self,
_label: str,
owner_is_member: bool,
expected_exception_type: str,
mock_export: MagicMock,
mock_uuid: MagicMock,
) -> None:
if owner_is_member:
self.organization.available_product_features = [
{"key": AvailableFeature.ACCESS_CONTROL, "name": AvailableFeature.ACCESS_CONTROL}
]
self.organization.save()
owner = User.objects.create_and_join(
self.organization, "member@example.com", None, level=OrganizationMembership.Level.MEMBER
)
AccessControl.objects.create(
team=self.team,
resource="dashboard",
resource_id=str(self.exported_asset.dashboard_id),
access_level="none",
)
else:
owner = User.objects.create_user(email="former-member@example.com", first_name="", password=None)
asset = ExportedAsset.objects.create(
team=self.team, dashboard=self.exported_asset.dashboard, export_format="image/png", created_by=owner
)

exporter.export_asset(asset.id)

mock_export.assert_not_called()
asset.refresh_from_db()
assert asset.exception_type == expected_exception_type
assert asset.failure_type == FAILURE_TYPE_USER


class TestExportAssetFailureRecording(APIBaseTest):
@parameterized.expand(
Expand Down
26 changes: 0 additions & 26 deletions posthog/temporal/alerts/activities.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,12 @@

from posthog.schema import AlertState

from posthog.hogql.errors import TableAccessDeniedError

from posthog.clickhouse.cancel import cancel_query_on_cluster
from posthog.clickhouse.query_tagging import Feature, Product, tag_queries
from posthog.dataclasses import frozen
from posthog.email import is_email_available
from posthog.errors import CH_TRANSIENT_ERRORS, CHQueryErrorQueryWasCancelled
from posthog.exceptions_capture import capture_exception
from posthog.query_creator_access import creator_access_revoked, report_creator_access_revoked
from posthog.schema_migrations.upgrade_manager import upgrade_insight
from posthog.sync import database_sync_to_async
from posthog.tasks.alerts.investigation_notifications import run_investigation_notification_safety_net
Expand Down Expand Up @@ -700,29 +697,6 @@ def _evaluate(alert: AlertConfiguration) -> EvaluateAlertResult:
# existing path instead of capturing it as an exception, which would pollute error
# tracking with a config problem that recurs on every check until fixed.
invalid_configuration = str(err)
except TableAccessDeniedError as err:
logger.exception("Alert failed to evaluate", alert_id=alert.id, exc_info=err)
# A revoked creator's access-denied error is a known limitation - report it as an event
# rather than capturing it, which would recur on every check until the alert is fixed.
if creator_access_revoked(alert.created_by, alert.team):
report_creator_access_revoked(
user=alert.created_by,
team=alert.team,
source="alert",
error=err,
properties={"alert_id": str(alert.id), "insight_id": alert.insight_id},
)
error = {"message": str(err)}
else:
capture_exception(
err,
additional_properties={
"alert_configuration_id": str(alert.id),
"insight_id": alert.insight_id,
"team_id": alert.team_id,
},
)
error = {"message": str(err), "traceback": traceback.format_exc()}
except Exception as err:
logger.exception("Alert failed to evaluate", alert_id=alert.id, exc_info=err)
capture_exception(
Expand Down
71 changes: 69 additions & 2 deletions posthog/temporal/tests/test_alerts_activities.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
TrendsQuery,
)

from posthog.hogql.errors import TableAccessDeniedError

from posthog.clickhouse.query_tagging import get_query_tags
from posthog.constants import AvailableFeature
from posthog.errors import CHQueryErrorQueryWasCancelled
Expand All @@ -38,7 +40,7 @@
ClickHouseClusterMemoryLimitExceeded,
ClickHouseQueryMemoryLimitExceeded,
)
from posthog.models import Team, User
from posthog.models import OrganizationMembership, Team, User
from posthog.redis import get_client
from posthog.slo.types import SloOperation, SloOutcome
from posthog.tasks.alerts.utils import (
Expand Down Expand Up @@ -75,7 +77,13 @@
SkipReason,
)

from products.alerts.backend.evaluation.contract import AlertDataUnavailableError, AlertExtractionError
from products.access_control.backend.models.access_control import AccessControl
from products.alerts.backend.evaluation.contract import (
CREATOR_LEFT_DISABLE_REASON,
CREATOR_LOST_ACCESS_DISABLE_REASON,
AlertDataUnavailableError,
AlertExtractionError,
)
from products.alerts.backend.evaluation.validation import THRESHOLD_BOUNDS_REQUIRED_MESSAGE
from products.alerts.backend.facade.api import (
LLM_DETECTOR_UNAVAILABLE_ERROR_CODE,
Expand Down Expand Up @@ -144,9 +152,14 @@ async def _create_alert(
insight_deleted: bool = False,
state: str = AlertState.NOT_FIRING,
detector_config: dict | None = None,
created_by: User | None = None,
) -> AlertConfiguration:
@sync_to_async
def _create() -> AlertConfiguration:
# Production always writes an owner, and an alert with none counts as one whose owner was deleted.
owner = created_by or User.objects.create_and_join(
ateam.organization, f"alert-owner-{uuid.uuid4().hex[:6]}@posthog.com", None
)
insight = Insight.objects.create(
team=ateam,
name="insight",
Expand Down Expand Up @@ -174,6 +187,7 @@ def _create() -> AlertConfiguration:
schedule_start_time=schedule_start_time,
state=state,
detector_config=detector_config,
created_by=owner,
)
return alert

Expand Down Expand Up @@ -1263,6 +1277,59 @@ async def test_evaluate_auto_disables_and_skips_error_tracking_on_configuration_
await sync_to_async(check.refresh_from_db)()
assert check.targets_notified

@pytest.mark.parametrize(
"owner_change, expected_message",
[
("left_organization", CREATOR_LEFT_DISABLE_REASON),
("lost_table_access", CREATOR_LOST_ACCESS_DISABLE_REASON),
("lost_insight_access", CREATOR_LOST_ACCESS_DISABLE_REASON),
],
)
async def test_evaluate_auto_disables_when_the_owner_can_no_longer_run_the_query(
self, alert_with_user, aorganization, owner_change, expected_message
) -> None:
owner = await sync_to_async(lambda: alert_with_user.subscribed_users.get())()
alert_with_user.created_by = owner
await sync_to_async(alert_with_user.save)(update_fields=["created_by"])
if owner_change == "left_organization":
await sync_to_async(
OrganizationMembership.objects.filter(organization_id=aorganization.id, user=owner).delete
)()
if owner_change == "lost_insight_access":
# The owner is a plain member, so a no-access rule on the insight applies to them.
aorganization.available_product_features = [
{"key": AvailableFeature.ACCESS_CONTROL, "name": AvailableFeature.ACCESS_CONTROL}
]
await sync_to_async(aorganization.save)()
await sync_to_async(AccessControl.objects.create)(
team=alert_with_user.team,
resource="insight",
resource_id=str(alert_with_user.insight_id),
access_level="none",
)

with (
# Patched under the real `check_alert_for_insight`, which turns the denial into the owner reason.
patch(
"products.alerts.backend.evaluation.dispatcher._check_alert_for_insight",
side_effect=TableAccessDeniedError("revenue"),
) as mock_check,
patch("posthog.temporal.alerts.activities.capture_exception") as mock_capture,
):
result = await ActivityEnvironment().run(
evaluate_alert, EvaluateAlertActivityInputs(alert_id=str(alert_with_user.id))
)

assert result.new_state == AlertState.ERRORED
mock_capture.assert_not_called()
# A departed owner's alert must stop even when its query would still run.
assert mock_check.called is (owner_change == "lost_table_access")
check = await sync_to_async(AlertCheck.objects.get)(pk=result.alert_check_id)
# The shared code routes the check to the "alert disabled" email, in-app notice and banner.
assert check.error == {"message": expected_message, "code": "invalid_configuration"}
refreshed = await sync_to_async(AlertConfiguration.objects.get)(pk=alert_with_user.pk)
assert refreshed.enabled is False

# Transient CH errors bubble up so Temporal's retry policy handles them.
# Capacity errors (codes 202/439) surface as ClickHouseAtCapacity, so that's what we simulate.
# A server-wide or per-user memory limit is the same kind of cluster pressure: recording it as
Expand Down
23 changes: 22 additions & 1 deletion posthog/test/test_query_creator_access.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
from posthog.test.base import BaseTest

from posthog.constants import AvailableFeature
from posthog.models import OrganizationMembership
from posthog.query_creator_access import creator_access_revoked
from posthog.query_creator_access import assert_creator_can_run, creator_access_revoked

from products.access_control.backend.facade.user_access_control import UserAccessControlError
from products.access_control.backend.models.access_control import AccessControl


class TestCreatorAccessRevoked(BaseTest):
Expand All @@ -19,3 +23,20 @@ def test_deactivated_owner_is_revoked(self) -> None:
def test_owner_removed_from_org_is_revoked(self) -> None:
OrganizationMembership.objects.filter(organization=self.organization, user=self.user).delete()
assert creator_access_revoked(self.user, self.team) is True

def test_member_without_project_access_lost_access_rather_than_left(self) -> None:
self.organization.available_product_features = [
{"key": AvailableFeature.ACCESS_CONTROL, "name": AvailableFeature.ACCESS_CONTROL}
]
self.organization.save()
# Admins keep access to a private project, so the owner is demoted to a plain member first.
self.organization_membership.level = OrganizationMembership.Level.MEMBER
self.organization_membership.save()
AccessControl.objects.create(
team=self.team, resource="project", resource_id=str(self.team.id), access_level="none"
)
# Still in the org, so not "left": the recipients' message must say access, not membership.
assert creator_access_revoked(self.user, self.team) is False
with self.assertRaises(UserAccessControlError) as raised:
assert_creator_can_run(self.user, self.team)
assert raised.exception.resource == "project"
12 changes: 12 additions & 0 deletions products/alerts/backend/evaluation/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ class AlertDataUnavailableError(AlertExtractionError):
"""This check lacks usable data; keep the alert enabled for future checks."""


class AlertOwnerAccessError(AlertExtractionError):
"""The query runs with the owner's access, and the owner can no longer run it.

Both evaluation stacks disable the alert for an `AlertExtractionError`, so this subclass only
lets a test or a log tell the owner case apart.
"""


CREATOR_LEFT_DISABLE_REASON = "The alert owner is no longer in the organization."
CREATOR_LOST_ACCESS_DISABLE_REASON = "The alert owner no longer has access to some of the data it reads."


def lookback_intervals_for(condition: AlertCondition) -> int:
"""How many trailing intervals an extractor must fetch for this condition.

Expand Down
Loading
Loading