fix(db): distinguish permission-denied discovery - #278
Open
matheuscoelhomalta wants to merge 2 commits into
Open
matheuscoelhomalta wants to merge 2 commits into
matheuscoelhomalta wants to merge 2 commits into
Conversation
Owner
|
Thanks for the PR @matheuscoelhomalta ! Please give me a day or two to get round to taking a look at this. |
ryanlewis
requested changes
Sep 25, 2026
ryanlewis
left a comment
Owner
There was a problem hiding this comment.
Thanks for this, @matheuscoelhomalta. It's a really useful fix. Getting "database not found" when macOS is actually refusing access is confusing, and the injected-failure tests are a nice way to cover it without touching TCC settings.
A few things before merging:
- A stray
ThingsData-*file now stops discovery. If a file (not a folder) matching the prefix is in the container,statreturns ENOTDIR. That falls through to thedefaultcase, so discovery fails.filepath.Globused to skip these. Could you treatsyscall.ENOTDIRlike not-exist and add a test for it? - README placement: the new
### Database diagnosticssection sits inside the Configuration section. The paragraph after it ("Anything set here still loses to a flag...") now reads as if it's about doctor. Could you move it to just before### Listing tasks? - Exit code: every other command exits non-zero on failure. I'd like
doctorto do the same: still print the full report to stdout, but exit 1 whenokis false. Thenthings doctor && ...works as you'd expect. Happy to talk about it if you feel strongly the other way. - Optional: if one
ThingsData-*folder is unreadable but another has the database, discovery now fails where it used to succeed. You could remember the permission error and only return it when nothing matched.
Everything else looks good to me. Thanks again!
Owner
|
Hi @matheuscoelhomalta, just checking in on this. main has moved on quite a bit since, so it'll need a rebase as well as the changes from my review. No rush, and if you'd rather I take it from here, just say so. |
This branch has not been deployed
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.
Summary
filepath.Globdatabase discovery with permission-aware directory traversalEPERM/EACCESduring discovery and database opening instead of reporting the database as missing or reducing the error to SQLite code 14things doctorwith plain and JSON diagnostics that do not query task dataWhy
filepath.Globreturns an empty match set when a process cannot enumerate Things' app-group container. The CLI therefore reportsThings3 database not foundeven when the database exists and the actual failure is a macOS privacy denial.This was observed on macOS 27 when
thingswas launched by a desktop agent process. The same database and v0.8.0 binary remained readable over SSH, so this change does not attempt to bypass macOS privacy controls; it makes the failure accurate and inspectable.things doctorreports automatic/config/flag selection, the inspected paths, a stable status such aspermission_denied, and whether the selected database opens read-only. A completed diagnosis exits successfully even whenokis false so JSON consumers always receive the report.The final commit was exercised in the affected macOS 27 desktop-agent context. Automatic discovery changed from the misleading
database not foundresult topermission_denied; an explicit--dbpath also reportspermission_deniedand preservesoperation not permittedinstead of reducing the cause toSQLITE_CANTOPEN (14). Normal task listing remains blocked by macOS in that context, as expected: this change diagnoses the privacy denial and does not bypass it.Verification
go test -race ./...golangci-lint v2.12.2 run ./...go build ./cmd/thingsthings --json doctoragainst a live Things databasethings --json list todayagainst the same database to confirm normal discovery remains functional--dbdiagnosis at commitecc2b98The permission-denied paths are covered with injected
readdir,stat, andopenfailures, so the tests do not depend on changing the runner's real macOS privacy settings.