fix(sqlite adapter): prevent path traversal via user-controlled dbPath

[sebastiondev]

Summary

The SQLite notification adapter (lib/notification/adapter/sqlite.js) uses the user-controlled dbPath field verbatim to create directories and open a database file. Because notification adapter fields are stored per-job by any authenticated user via POST /api/jobs, a non-admin user can supply an arbitrary absolute path or a ../-containing relative path and cause Fredy to:

• create arbitrary directories (via fs.mkdirSync(dbDir, { recursive: true })), and
• create/open a SQLite database file at any location writable by the Fredy process (via new Database(dbPath)).

This is CWE-22 (Path Traversal / arbitrary file & directory creation). Severity is moderate: it requires an authenticated account, but Fredy explicitly supports multiple non-admin users, and this capability goes well beyond what the UI is meant to expose (writing outside the app directory, potentially clobbering unrelated files, or seeding files into sensitive locations like ~/.ssh/, cron directories, or a reverse-proxy webroot depending on how the container/host is provisioned).

Data flow

1. POST /api/jobs accepts a job payload including notificationAdapter[].fields.dbPath from any authenticated session (see authHook in lib/api).
2. The job is persisted to the jobs store.
3. On job execution, send() in lib/notification/adapter/sqlite.js reads sqliteConfig?.fields?.dbPath and passes it directly to path.dirname, fs.mkdirSync, and new Database().

No sanitisation, base-directory containment, or absolute-path rejection existed on this path prior to the fix.

Fix

Introduce resolveSafeDbPath() which:

• treats the configured value as a path relative to process.cwd() (the Fredy application directory), defaulting to db/listings.db when unset/blank,
• rejects absolute paths outright,
• resolves the final path with path.resolve and verifies it is either equal to the base directory or starts with baseDir + path.sep (the trailing-separator check is important — a naive startsWith(baseDir) would allow /app-evil to bypass a /app prefix check),
• throws a clear error on violation so the notifier fails loudly instead of silently writing outside the sandbox.

The adapter's field description is also updated so the UI communicates the new constraint.

The change is contained to a single file (28 insertions, 2 deletions) and preserves backward compatibility for the documented, intended usage (db/listings.db or other relative paths under the app directory).

Proof of concept

As any authenticated non-admin user:

# 1) Log in and grab the session cookie (replace creds).
curl -c cookies.txt -X POST http://localhost:9998/api/login \
  -H 'Content-Type: application/json' \
  -d '{"username":"user","password":"password"}'

# 2) Create a job whose sqlite adapter writes outside the app directory.
curl -b cookies.txt -X POST http://localhost:9998/api/jobs \
  -H 'Content-Type: application/json' \
  -d '{
        "name":"pwn",
        "provider":[],
        "notificationAdapter":[
          {"id":"sqlite","fields":{"dbPath":"/tmp/fredy-pwn/evil.db"}}
        ]
      }'

# 3) When the job next runs (or is triggered), before the fix Fredy calls
#    fs.mkdirSync('/tmp/fredy-pwn', {recursive:true}) and creates the DB file.
#    After the fix, resolveSafeDbPath() throws and no filesystem write occurs
#    outside the application directory.

Relative traversal ("dbPath":"../../etc/cron.d/evil.db") is likewise blocked because path.resolve(process.cwd(), '../../etc/cron.d/evil.db') lands outside process.cwd() and fails the containment check.

Testing

• Verified the vulnerable path by reading lib/notification/adapter/sqlite.js before the change: sqliteConfig?.fields?.dbPath flowed unchanged into fs.mkdirSync and new Database.
• Verified the fix locally by exercising resolveSafeDbPath() with:
  • unset / empty → resolves to <cwd>/db/listings.db (unchanged default behaviour),
  • "db/listings.db" → resolves inside cwd (accepted),
  • "/etc/passwd" → rejected (absolute path),
  • "../../etc/cron.d/x" → rejected (escapes cwd),
  • "db/../db/listings.db" → accepted (normalises inside cwd).
• git diff master..HEAD --stat shows the change is isolated to one file.

Adversarial review

Before submitting we tried to talk ourselves out of this. The endpoint is authenticated, so is it really a boundary crossing? Yes: Fredy documents and implements a distinction between admin and non-admin users, and neither role is intended to have arbitrary filesystem write on the host. We also considered whether the notification adapter is only reachable to admins — it isn't; the job/adapter APIs are gated by the generic session authHook, not an admin check. Finally, we considered whether better-sqlite3 itself normalises or restricts the path — it doesn't; it opens whatever path you give it and will happily create the file. The containment check is the right layer for this fix.

---

_{Discovered by the Sebastion AI GitHub App.}

[orangecoding]

We had one person in the past who created exactly the same pr and we rejected it, because this change (line 25) can and will break existing installations using this adapter if they have an absolute path.

What's your take on this

[sebastiondev]

Thanks for the context — that's a fair objection, and you're right that a hard rejection of absolute paths is a breaking change for anyone who legitimately configured e.g. /var/lib/fredy/listings.db. I don't want to force that migration on existing installs.

The reason I still think there's something worth fixing here is the trust boundary rather than the path shape itself: notificationAdapter[].fields.dbPath is settable by any authenticated user on POST /api/jobs (the route is gated by the generic session authHook, not an admin check), so a non-admin can currently make Fredy fs.mkdirSync and open a SQLite file at any location writable by the process — /tmp/x, an SSH config directory, a reverse-proxy webroot, etc. That's what I was trying to contain.

Given your constraint, some options that preserve existing installs:

1. Admin-only for absolute paths. Allow absolute paths only when the job is being created/edited by an admin; non-admins get relative-only. Backward compatible for anyone who set it up as admin (the normal case), and closes the non-admin escalation.
2. Opt-in allowlist via env var. e.g. FREDY_SQLITE_ALLOWED_DIRS=/var/lib/fredy:/data. Default = cwd only. Existing installs add one env var on upgrade; nothing silently breaks because the error tells them exactly what to set.
3. Restrict at the API layer, not the adapter. Leave the adapter permissive (no behavioural change for existing DBs), but reject dbPath changes from non-admin users in the jobs route. Zero impact on existing configs; the fix lives entirely in the authorization layer.

My preference would be (3) — smallest behavioural change, doesn't touch the adapter at all, and matches the actual problem (authorization, not path validation). Happy to rework the PR that way, or (1) if you'd rather keep the check next to the adapter. Or if you'd prefer to leave this as-is and treat "any authenticated user can configure adapters" as intended, I'm fine closing the PR — your call.