feat: set SESSION_CONTEXT per request for Row-Level Security - #6
Open
lbertrand wants to merge 1 commit into
Open
feat: set SESSION_CONTEXT per request for Row-Level Security#6lbertrand wants to merge 1 commit into
lbertrand wants to merge 1 commit into
Conversation
Tables guarded by Row-Level Security usually scope rows via SESSION_CONTEXT() rather than by login, since the server connects with one shared account. The query validator blocks EXEC/SP_, so the context could never be set and every RLS-protected query came back empty with no indication why. SESSION_CONTEXT is connection-scoped and queries run through a pool, so the context is emitted in the same batch as the query rather than in a separate round trip that would land on an arbitrary connection. The preamble is built from process config and bound via request.input(), so no part of it derives from model-supplied input and security.ts is unchanged. Keys are locked with @read_only = 1 and each EXEC is guarded on SESSION_CONTEXT() IS NULL, which keeps it idempotent across reused pooled connections (re-setting a locked key raises 15664). resetSessionContext() recycles the pool for the case where a locked value must change. When SQLSERVER_SESSION_CONTEXT is unset the emitted SQL is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5.
Tables guarded by Row-Level Security usually scope rows through
SESSION_CONTEXT()rather than by login, because the server connects with a single shared account.QueryValidatorblocksEXEC,EXECUTEandSP_, so the context could never be established and every RLS-protected query returned an empty recordset with nothing to indicate why. #5 has the full reasoning; this is the implementation.Approach
SESSION_CONTEXTis connection-scoped and queries run through a pool of up to 10, so the context is emitted in the same batch as the query rather than in a separate round trip that would land on whichever connection the pool handed out.applySessionContext()builds the preamble and binds its parameters;query()prepends it.For
SQLSERVER_SESSION_CONTEXT='{"TenantId":42,"UserRole":"auditor"}'the batch is:Notes for review
security.tsis untouched. The preamble is built from process configuration and never from thequeryparameter, so the read-only guarantee on model-supplied SQL is unchanged and the validator needs no exception. This should not conflict with the validator work in Improved Forbidden Keywords #1 or Security fixes #3.query(), afteraddRowLimit()has run — its/^(\s*SELECT\s+)/ianchor would stop matching otherwise.@read_only = 1is the default. A query then cannot widen its own scope even if it reached the server with context already set. EachEXECis guarded onSESSION_CONTEXT(@key) IS NULLto stay idempotent across reused pooled connections, since re-setting a locked key raises error 15664.resetSessionContext()recycles the pool for the one case the guard cannot cover: changing a value that was locked read-only. Read-only keys are released only on connection reset; tedious exposesConnection.reset, butmssqlnever calls it and offers no per-checkout hook, so pool recycling is the only route through the public API.SQLSERVER_SESSION_CONTEXT_READONLY=falseopts out.intso predicates comparing against integer columns work without aCONVERT; strings bind asnvarchar.Compatibility
When
SQLSERVER_SESSION_CONTEXTis unset, the preamble is empty and the emitted SQL is byte-identical to today's. Both new settings are optional.Verification
npx tsc --noEmitis clean. Behaviour was exercised against a stubbed pool capturing the emitted batch and bound parameters:EXECper key,42bound asInt,"auditor"asNVarCharSQLSERVER_SESSION_CONTEXT_READONLY=false→ unguardedEXEC, no@read_onlySELECT 1emitted verbatim, no preamblenot json,[1,2], nested objects) → rejected at startup with a message naming the expected shapeI did not add a test file:
npm testrunsjestbut there is no jest config orsrc/test/onmain(#3 adds both). Glad to add coverage in whichever form you'd prefer, or to rebase on #3 once it lands.