Skip to content

A token reads a document's extractions and exports its base - #1266

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/token-reads-extractions-and-export
Oct 23, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/token-reads-extractions-and-export

Conversation

@gedaiu

@gedaiu gedaiu commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Part of #1262, and the last of its reads.

What happened

A backend that mirrors into a base could not read what extraction made of a document it pushed, and it could not take the base out as RDF. Both reads took a session JWT, so a personal token got 401.

What changed

GET /documents/{id}/extractions and GET /kbs/{id}/export take SessionOrToken and call require_reader_kb (0052): the owner needs Viewer, any token scope may read, and a token whose kb_ids exclude the base gets 404. Extractions resolves the document's base first, as content does. Export still writes kb.exported with the owner as actor, and when a token was used the row's detail also carries the token id through audit::via_token. require_reader_kb now returns the base it loaded, so export no longer needs a second kbs::get.

Decision 7 in 0014 gets a short paragraph on the two reads, including why a read token may export the whole base: a Viewer could already copy it out piece by piece through the other reads, write scope would only push a mirror to hold a write token for a read-only job, and the audit row naming the token is the control. The revision line gains one sentence and #1262 joins the status line. The SessionOrToken comment in auth.rs and docs/design/access-and-audit.md name both reads.

Tests

Both GETs join read_paths in api::token_read_tests, using a document added to the shared fixture: a viewer's read token gets 200 and a token limited to another base gets 404. A separate test checks that an export by token writes kb.exported with the owner as actor and the token id in the detail. Switching either route back to session-only fails the shared tests with 401, and dropping via_token fails the audit test.

cargo test -p utopia-server against Postgres with UTOPIA_TEST_REQUIRE_DB=1, cargo fmt --all -- --check and cargo clippy -p utopia-server --all-targets -- -D warnings are clean.

🤖 Generated with Claude Code

@WaylandYang WaylandYang 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.

Thank you. Both routes are GETs behind require_reader_kb. Extractions finds the document's base first, as content does, so a token outside kb_ids gets the same 404 whether or not the document exists, and the facts it returns stay in that base because 0070's fact_evidence_same_kb trigger keeps each evidence row's chunk and fact together. Export is a read-only snapshot and starts no work; its one write is the kb.exported row, which now names the token through audit::via_token and is written only after the check passes. Both tests run in CI and fail without the change.

Before merge:

  1. Please rebase on #1263, #1264 and #1265 once they're in, and fold these two routes into decision 7 ("A token reads the graph") with their reasons. Extend its revision line rather than adding a decision and a revision line of your own; the blank line before the new entry also splits the list. If this ends up the last of the read PRs, it is the one that adds #1262 to the status line.
  2. Please add extractions and export to the SessionOrToken comment in auth.rs (line 179 on dev), so that list stays the place where a reader learns which routes a token reaches.
  3. In decision 7, please say in one sentence why a read token may export the whole base. The route comment in api/mod.rs already gives the reason for a Viewer: anything a Viewer sees could be copied out one piece at a time, and with #1263–#1265 in, the same holds for a token. Requiring write scope would only push a mirror to hold a write token for a read-only job, and the audit row naming the token is the control. That reasoning relies on the other reads being open, so this PR should go in last.

Two optional things: token_read_tests from #1263 could take these two GETs instead of a fourth copy of the fixture, and require_reader_kb could return the base it already loaded, which would save the second kbs::get in export.

@gedaiu

gedaiu commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, agreed it goes last. After #1263–#1265 I'll rebase, fold extractions and export into decision 7 with one sentence on why a read token may export (a Viewer could copy it out piece by piece through the other reads; a write scope would only push a mirror to hold a write token for a read-only job; the audit row naming the token is the control), extend the revision line without the stray blank line, add both to the auth.rs list, add #1262 to the status line, move the tests into token_read_tests, and have require_reader_kb return the base it loaded so export drops its second kbs::get.

@gedaiu

gedaiu commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Moving this to draft so only one of the #1262 reads is open at a time, as you asked there. Once #1263 is in I'll rebase this onto it, fold the routes into decision 7 and the shared token_read_tests fixture as agreed above, and mark it ready again. Thanks.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Bogdan Szabo <[email protected]>
@gedaiu
gedaiu force-pushed the feat/token-reads-extractions-and-export branch from ed4b9f9 to 155a6dd Compare October 22, 2026 12:56
@gedaiu
gedaiu marked this pull request as ready for review October 22, 2026 12:56
@gedaiu

gedaiu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Thank you @WaylandYang, both for the careful review and for writing out the export reasoning, which I used almost as you put it. Now that #1265 is in, I've rebased onto dev:

  1. Extractions and export are in decision 7 in a short paragraph after the document read from A client reads a document back by the external_id it pushed it under #1280, with the revision line extended by one sentence. As the last of the reads, it adds A personal token could read the graph of the bases it reaches #1262 to the status line.
  2. auth.rs and access-and-audit.md list both reads.
  3. The paragraph says why a read token may export the whole base. The kb.exported row naming the token is the control.

Both GETs sit in read_paths in token_read_tests, with one small test for the audit row. require_reader_kb now returns the base it loaded, so export drops its second kbs::get. Ready for another look whenever suits you.

@WaylandYang WaylandYang 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.

Thank you, all three points are in, and both optional ones too. The export reasoning in decision 7 reads right. Merging.

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.

2 participants