Skip to content

Enforce table read permissions for subscriptions on Harper 5.1#1914

Open
kriszyp wants to merge 2 commits into
v5.1from
fix/v5.1-subscribe-allowread
Open

Enforce table read permissions for subscriptions on Harper 5.1#1914
kriszyp wants to merge 2 commits into
v5.1from
fix/v5.1-subscribe-allowread

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 23, 2026

Copy link
Copy Markdown
Member

Ensures tables using loadAsInstance = false invoke allowRead before subscription setup, matching the existing get authorization contract and preventing checked subscriptions from bypassing table read permissions. This intentionally preserves v5.1 table-level semantics instead of backporting main’s row-level delivery behavior; generated by GPT-5.6 Codex.

Attention: cross-model coverage was Gemini ✓ / Harper domain pass ✓ / Codex CLI timeout. The full integration matrix reached 1,321 passes and zero assertion failures; its command exited nonzero only because the unrelated real-Ollama fixture hit Node ERR_IMPORT_ATTRIBUTE_MISSING while loading json/systemSchema.json and cancelled six children.

@kriszyp
kriszyp requested review from cb1kenobi and heskew July 23, 2026 18:37

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces permission checks to the table subscription process in resources/Table.ts and adds corresponding unit tests in permissions.test.js. The review feedback recommends guarding against potential nullish context values using optional chaining to prevent runtime errors, and suggests using assert.strictEqual instead of assert.equal in the test suite to align with the repository's style guide.

Comment thread resources/Table.ts Outdated
Comment thread unitTests/resources/permissions.test.js Outdated
@kriszyp

kriszyp commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

Main/5.2 follow-up with full connection + per-record authorization semantics: #1915

@kriszyp
kriszyp requested review from Devin-Holland and removed request for cb1kenobi July 23, 2026 19:03
@kriszyp
kriszyp marked this pull request as ready for review July 23, 2026 23:59
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