Skip to content

adds set_authorizer/2 (fixes #344)#345

Merged
warmwaffles merged 2 commits intoelixir-sqlite:mainfrom
WhiskeyTuesday:add-set-authorizer
Mar 27, 2026
Merged

adds set_authorizer/2 (fixes #344)#345
warmwaffles merged 2 commits intoelixir-sqlite:mainfrom
WhiskeyTuesday:add-set-authorizer

Conversation

@WhiskeyTuesday
Copy link
Copy Markdown
Contributor

Adds Exqlite.Sqlite3.set_authorizer/2 which registers a deny-list authorizer via sqlite3_set_authorizer(). The caller passes a list of action atoms to block (e.g. [:attach, :detach]), and a static C callback denies those action codes during sqlite3_prepare(). Pass an empty list to clear the authorizer.

:ok = Exqlite.Sqlite3.set_authorizer(conn, [:attach, :detach])
{:error, "not authorized"} = Exqlite.Sqlite3.execute(conn, "ATTACH DATABASE 'other.db' AS x")

:ok = Exqlite.Sqlite3.set_authorizer(conn, [])  # clear

A full Erlang callback isn't practical here since the authorizer is called synchronously during sqlite3_prepare() and must return immediately. The deny-list approach avoids this as config is stored in the connection struct as a flat int[64] array indexed by action code and the callback is a single bounds check and array lookup with no allocation.

Implementation notes:

  • Follows the same pattern as set_update_hook
  • All 31 SQLite action codes (1-33) are mapped to snake_case atoms
  • The deny array is sized to 64 for headroom beyond the current max action code (33)
  • Invalid atoms in the deny list raise ArgumentError without modifying the existing authorizer
  • 7 tests: deny single, deny multiple, allow non-denied, clear, high action code (savepoint), invalid atom preserves existing authorizer, pure invalid raises

Since this feature is likely to be used in security-sensitive contexts I'd appreciate a careful review of the C code. I'm not the most experienced C developer and this touches the security boundary of the library.

PR description written mostly by AI so if it sounds like slop it's because it is

Comment thread c_src/sqlite3_nif.c Outdated
Comment thread c_src/sqlite3_nif.c
Comment on lines +51 to +53
// Denied authorizer action codes. Sized to 64 for margin — highest
// currently defined SQLite action code is SQLITE_RECURSIVE (33).
#define AUTHORIZER_DENY_SIZE 64
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you able to come up with a test that reaches the max deny size of 33? I just want to ensure a segfault does not occur.

@WhiskeyTuesday
Copy link
Copy Markdown
Contributor Author

Added braces and an array exhaustion test that denies all 31 action codes at once including :recursive (code 33, the highest index). This way if someone shrinks AUTHORIZER_DENY_SIZE below 34 the test fails in CI rather than silently passing and segfaulting at runtime.

The list can't be generated programmatically since the atom-to-code mapping lives in C and the test in Elixir, but the SQLite action code list hasn't changed since 3.8.3 (2014). And if a new code is ever added, action_code_from_atom needs updating anyway.

@WhiskeyTuesday
Copy link
Copy Markdown
Contributor Author

The list can't be generated programmatically

I mean, anything is possible but we'd have to pass a shared source of truth across the NIF barrier and it would be code that exists only for testing purposes

Copy link
Copy Markdown
Member

@warmwaffles warmwaffles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anything wrong with this. Thank you for implementing.

@warmwaffles
Copy link
Copy Markdown
Member

warmwaffles commented Mar 27, 2026

We could static assert that the AUTHORIZER_DENY_SIZE is always greater than or equal to what is in sqlite, but the codes we check against are fixed. Inevitably someone would complain if a new one was added and we didn't support it. Problem for later.

@warmwaffles
Copy link
Copy Markdown
Member

Also I appreciate the honesty about he AI generation 😉

@warmwaffles
Copy link
Copy Markdown
Member

I'm gonna merge this and toy with it some. I'll ping back here when I cut a release for it.

@warmwaffles warmwaffles merged commit 2f5a524 into elixir-sqlite:main Mar 27, 2026
11 checks passed
@warmwaffles
Copy link
Copy Markdown
Member

Memory consumption wise, we could probably use a 64 bit unsigned integer and bit shift with flags.

@warmwaffles
Copy link
Copy Markdown
Member

Made some corrections, and will be released under v0.36.0

@WhiskeyTuesday
Copy link
Copy Markdown
Contributor Author

Sounds great, I didn't have time to think about your other comments yet but you know this codebase infinitely better than I do so I'm sure it'll work 😸

@warmwaffles
Copy link
Copy Markdown
Member

I'll leave the bit shifting thing until later. There's some overhead, ever so slight by authorizing every action and jumping the list in memory, as opposed to a bit check. Let me know if you see degraded performance.

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