Skip to content

bug: Experience reads can return content written after custom-policy revocation #2017

Closed

Description

@Zxf-xufeng

Describe the bug

An in-flight shared Experience read on SQLite can return a revision written after access was revoked when a configured custom authorization provider does not read through the content database connection.

The runtime enters a consistent_snapshot=True transaction, but SQLite has not pinned its content snapshot before service.get() checks authorization. The first Artifact table read happens afterward. An allowed decision captured before revocation can therefore be combined with content committed after revocation.

This is a standalone follow-up to the reviews of #1882. The final review confirmed that the defect also existed in that PR's base; it is not a new regression introduced by #1882.

Evidence:

Steps to reproduce

Use actual SQLite with independent reader/writer connections and a controlled custom authorization provider. The provider must not incidentally pin the reader's content snapshot by reading from its connection.

  1. Create an Experience with revision 1 and allow the reader to access it.
  2. Start a read through runtime.artifact_merge.for_scope(scope, "experience").get(...).
  3. In the custom provider, capture an allowed authorization decision and pause before returning it. Use synchronization barriers to ensure that Artifact content has not yet been read.
  4. While the read is paused, revoke the reader's access and commit a replacement revision 2 through a separate writer connection.
  5. Resume the provider with the previously captured allowed decision and observe the in-flight read's result.
  6. Perform a fresh read after revocation.

The linked review reports this exact interleaving with a local custom-provider probe. It does not provide a complete standalone script; these steps describe the required deterministic regression scenario.

Expected behavior

An authorization decision captured before revocation must not authorize content written after revocation. Under the existing snapshot-read model, the in-flight read should return the pinned revision 1. Rejecting the read after revalidation would also avoid disclosure, but returning revision 2 must not be possible. A fresh read after revocation should be denied.

Pin the content snapshot before the policy check while preserving the configured custom provider and deferred audit behavior. A regression test should coordinate the interleaving explicitly and assert the returned revision as well as the denial of the subsequent read.

Actual behavior

The paused read returns revision 2, which was written after access was revoked. A fresh read correctly raises AccessDeniedError.

In the linked review's control, adding only an Artifact-head table read before authorization caused the same interleaving to return revision 1. Entering the transaction/savepoint alone did not establish that content snapshot.

Environment

  • Database in the original reproduction: actual SQLite.
  • Authorization: controlled local custom provider; not an external policy-service or HTTP-route test.
  • Last reviewed PR head: b402e590ab9244897d91123fc482e0c0cc770a9a.
  • Base where the same defect was confirmed: a72f223b88a341162618686a404e29d5f2290994.
  • The same authorization-before-content-read path was found during post-merge inspection of master on October 15, 2026.
  • Exact SQLite, Python, and operating-system versions were not specified in the linked review.
  • This report was drafted with OpenAI Codex from the linked reproduction evidence and source inspection. No new concurrency reproduction was run while filing it.

Are you willing to submit a PR to fix this bug?

  • Yes, I would like to submit a PR.

Activity

  1. MaoMengww commented on Oct 16, 2026

    @MaoMengww
    Contributor

    plz assign to me

  2. moved this from Todo to In progress in PowerContexton Oct 16, 2026
  3. tlyyxjz commented on Oct 16, 2026

    @tlyyxjz
    Contributor

    Fixed in #2026.

    The snapshot is the part that bites here. SQLite takes its read snapshot at the first statement that reads the database file, not at SAVEPOINT, and on this path that first read came after the authorization check. So an allowed decision could be paired with content that was committed after the revocation.

    #2026 reads one Artifact head row as the first statement of the consistent_snapshot=True blocks in artifact_merge.py, which is the same read the pin sites in atomic_memory.py and atomic_memory_processing.py use. MySQL is not affected. There is a regression test that fails without the change (assert 2 == 1).

  4. tlyyxjz commented on Oct 16, 2026

    @tlyyxjz
    Contributor

    I opened #2026 about an hour after this was assigned here and only saw the assignment afterwards. That is on me.

    @MaoMengww if you already have a fix in flight, say so and I will close mine. Otherwise it is ready for review. The regression test is worth keeping either way: it drives the interleaving with a provider that uses its own connection, and it fails without the change.

  5. MaoMengww commented on Oct 16, 2026

    @MaoMengww
    Contributor

    @tlyyxjz It's okay, just go with yours.

  6. tlyyxjz commented on Oct 16, 2026

    @tlyyxjz
    Contributor

    Thanks @MaoMengww. If you happen to know whether the two other unpinned snapshot sites (atomic_memory_processing.py:547, relational.py:1040) can hit the same interleaving, I would like to hear it.

  7. tlyyxjz commented on Oct 16, 2026

    @tlyyxjz
    Contributor

    Scratch my question above, @MaoMengww. I went back and read both sites myself.

    • relational.py:1040 does pin. It reads one Artifact head row at line 1044, before access.require_scope_read(), with the comment "Establish the data snapshot before a remote PDP can suspend." I had it wrong earlier: the read sits inside an if and I filed it as business data without opening the line.
    • atomic_memory_processing.py:547 does not pin, and I do not think it needs to. That block only runs enumerate_related, the security filters it passes in are read in _read_transaction(), which does pin, and no policy call happens inside the block.

    So the count is 11 consistent_snapshot=True sites: 7 already pinned before this change, 3 added in #2026, 1 left alone for the reason above. I have corrected the PR description.

    If your reading of 547 is different, I would like to hear it.

  8. moved this from In progress to Done in PowerContexton Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions