Repository navigation
Conversation
|
Review requested:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #62957 +/- ##
==========================================
- Coverage 90.58% 90.51% -0.07%
==========================================
Files 797 799 +2
Lines 278480 279089 +609
Branches 53532 53671 +139
==========================================
+ Hits 252268 252629 +361
- Misses 16541 16727 +186
- Partials 9671 9733 +62
🚀 New features to boost your workflow:
|
8ffad93 to
877530d
Compare
RafaelGSS
left a comment
There was a problem hiding this comment.
As long as it doesn't open precedence for reports targeting SQLite on permission model, I'm +1.
There was a problem hiding this comment.
Hmm, I think this needs to take into account https://sqlite.org/uri.html
- A database can be opened read-only with
mode=roorimmutable=1. - Another way to create an in-memory database is with
mode=memory.
Doing more string processing on the database path sounds a bit hacky though.
Edit: looks like we have not enabled SQLITE_USE_URI=1, but it would be good to leave a comment here that this needs to be updated when we do at the very least.
As long as it doesn't open precedence for reports targeting SQLite on permission model, I'm +1.
Passing a custom VFS to SQLite that can really prevent reads/writes might be a more comprehensive solution worth considering...
https://sqlite.org/vfs.html sounds really cool! I will take a look. |
2bef788 to
b1cfecf
Compare
|
@mcollina This needs a rebase, and the first commit needs to be signed. |
Add permission model enforcement to DatabaseSync::Open(). File-backed databases now check kFileSystemRead (readOnly) or kFileSystemWrite (read-write) before calling sqlite3_open_v2. In-memory databases (:memory:) are exempt. Refs: https://hackerone.com/reports/3686625
Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
b1cfecf to
191231b
Compare
Deny ATTACH under the permission model when SQLite does not pass the filename to the authorizer. SQLite only does so for string literals, so a bound or computed filename could select a URI VFS and bypass the permission VFS. Resolve VFS paths lexically, as the permission model does, instead of following symbolic links, and match SQLite's exact super-journal name format. Share one permission VFS per Environment, release it when open() fails, skip the authorizer guards when no JavaScript callback is set, check each path once, and reuse FSPermission::is_granted() for side-effect-free checks. Assisted-by: Claude Code Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
1f9b8e3 to
e630e6b
Compare
Enforce the permission model for
node:sqlitefile access.open()checks--allow-fs-readfor read-only databases, and read plus write for read-write ones. In-memory databases are exempt.ATTACHtargets, backup destinations, and journal, WAL and SHM files. A database grant also covers its own sidecar files.vfs=parameters are rejected, so a connection cannot switch to an unchecked VFS.