dashlibstorage/TODO.md

TODO — Database provider reorganization

Milestones 1–3 are done (provider contract, MSSQL provider, repo reorg). SQLite provider, native schema, and query translations now exist; store wiring, migrations, lifecycle methods, and parity tests remain outstanding.

1. Convert store modules to the provider API (per area, keep behavior)

Replace raw pool usage in src/store/** with provider.query() / provider.transaction() / provider.exec() and the normalized result shape ({ rows, rowsAffected, insertId } instead of recordset / rowsAffected[]). For every module:

• pool.request().input(...).query(SQL) → provider.query(SQL, params)
• pool.transaction(); begin; commit; catch rollback → provider.transaction(async (tx) => { ... })
• result.recordset → result.rows; result.rowsAffected[0] → result.rowsAffected
• request.js helper ((tx ?? pool).request()) becomes obsolete — deleted
• src/store/runner.js bridges mixed call sites: converted modules accept a provider runner or a legacy mssql Transaction/pool (wrapped on the fly) so partially converted callers keep working
• recordset[0].id after OUTPUT INSERTED.id → keep, but prefer insertId where trivial

Conversion order (bottom-up by dependency):

• lang, tax (leaf helpers, used by company create)
• google_auth
• companyuser (resolveCompanyAccess / resolveSelectedCompanyAccess first)
• company, companysetting
• customergroup, entity, entityaddress, entitycontact, entityhistory
• unit (incl. ensureDefault), brand, supplier
• numbers (see §2 — stored procs!)
• category (recursive CTEs!)
• article + articletext/articlepic/articleean/price/articlehistory
• articlesupplier
• quote, order, delivery (shared manifest helpers: manifestLines, manifestDoc, manifestattachment)
• posterminal, pic
• initialize.js, purge.js final wiring (drop provider.pool fallback from index.js)
• Delete src/store/request.js once no module uses it

2. Replace MSSQL stored procedures (blocker for SQLite)

• numbers.nextNumber uses proc getnextnumberwithprefixandpadding (output param)
• numbers.peekNumber uses proc peeknextnumberwithprefixandpadding (output param)
• Implement both in plain JS inside the store layer: SELECT next/prefix/padding, compute prefix + padded next, UPDATE numbers SET next = next + 1 WHERE .... Must stay atomic: use provider.transaction() with row locking semantics (MSSQL: WITH (UPDLOCK, ROWLOCK) in the SELECT; SQLite: within its write tx).
• Drop the proc definitions from schema/init scripts (scripts/init-schema.js).

3. provider contract additions / hygiene

• Add isUniqueConstraintError / isForeignKeyConstraintError to the contract and use them in stores instead of raw mssql error numbers (currently error messages like Customer group in use, Brand in use, Unit in use rely on FK/unique detection — grep for these).
• Make initialize()/purge() real provider methods (schema migrations move into the provider; store delegates). Decide per §5.
• Document provider contract (JSDoc typedef in src/providers/provider.js).

4. better-sqlite3 provider

• Add src/providers/better-sqlite3/index.js implementing the same contract:
  • query() → prepare + .all()/.get()/.run(); normalize lastInsertRowid → insertId, changes → rowsAffected
  • transaction(fn) → db.transaction() wrapper (note: better-sqlite3 is sync; wrap in async signature so store code stays unchanged)
  • Named params: mssql uses @name; better-sqlite3 uses :name / @name / $name — define one (e.g. @name) in SQLite SQL so params pass through
  • Pragmas on open: foreign_keys = ON, journal_mode = WAL, busy_timeout = 5000
  • BLOB handling: return Buffer for pic/image/attachment binaries
  • Error classification: unique → SQLITE_CONSTRAINT_UNIQUE, FK → SQLITE_CONSTRAINT_FOREIGNKEY
  • Do NOT close the caller-owned db in the provider; expose close()
• better-sqlite3 as peerDependency (optional) in package.json
• package.json exports: "./providers/better-sqlite3"

5. SQLite schema (native, not mechanically translated)

• Native schema in src/providers/sqlite3/schema.js:
  • INT IDENTITY → INTEGER PRIMARY KEY (rowid)
  • NVARCHAR(n) → TEXT; VARBINARY(MAX) → BLOB
  • DATETIME2 DEFAULT SYSUTCDATETIME() → TEXT ISO-8601 UTC (strftime('%Y-%m-%dT%H:%M:%fZ','now'))
  • IF NOT EXISTS ... sys.tables → CREATE TABLE IF NOT EXISTS
  • OUTPUT INSERTED... in queries → RETURNING ...
  • MERGE companysetting → INSERT ... ON CONFLICT ... DO UPDATE
  • [identifier] brackets → rename or quote ("number")
• Add versioned SQLite migrations around the native schema
• Entity number uniqueness parity (MSSQL still uses indexed view v_entity_company_number): - [x] SQLite: add company_id to entity (denormalized from customergroup, kept in sync on insert + customergroup moves) and enforce CREATE UNIQUE INDEX uq_entity_company_number ON entity(company_id, number) WHERE number IS NOT NULL - [ ] Add the equivalent MSSQL migration for parity; remove the indexed view
• Versioned migrations via meta table (already exists) instead of pure IF NOT EXISTS scripts for both providers
• Implement initialize() (migrations) and purge() (drop tables, FK-order) in the SQLite provider; port purgeSql.js logic (currently topological drop order — replicate)

6. SQL translation per area (SQLite queries)

• Add all 32 modules under src/providers/sqlite3/queries/, preserving the MSSQL query modules' named-export API (plus SQLite number helpers)
• TOP 1 → LIMIT 1 (mind: TOP without ORDER BY — keep identical ordering)
• OUTER APPLY (SELECT TOP 1 ...) (article/category first-pic joins) → correlated subquery or JOIN on min(sort,id)
• FOR JSON PATH queries → SQLite JSON1 aggregation with shapes compatible with the current store parsers (building JSON in JS remains a future cleanup)
• Recursive CTEs (SEARCH_ARTICLES, RESOLVE_ARTICLE_ACCESS_BY_SKU, GET_CATEGORY_TREE_FLAT) → SQLite WITH RECURSIVE (mostly compatible)
• ISNULL / COALESCE differences; FLOAT vs REAL
• Case-insensitive comparisons relying on MSSQL default collation (e.g. name-uniqueness checks) → COLLATE NOCASE in SQLite schema or explicit LOWER() in both dialects
• Compile all 212 complete SQLite query exports against an in-memory native schema (zero syntax/schema errors)

7. Contract / parity tests

• Test harness that runs the identical suite against both providers (SQLite in-memory for speed; MSSQL in CI)
• Cover: return shapes, auth errors (No company selected, Not authorized), domain errors, tx rollback, FK/unique behavior (Brand in use etc.), binary round-trips (pic, manifestattachment, google_auth picturebinary), date handling, recursive category tree, importRows two-pass V/K linking, numbers.next/peek atomicity
• scripts/test-api.js: parametrize over provider
• scripts/init-schema.js, reset-db.js, purge-db.js: accept provider selection (e.g. --provider=better-sqlite3 / env DASH_DB_PROVIDER)

8. Docs & tooling

• README: provider-based usage, both examples, migration notes for dashback
• API.md / API.*.md generators (generate-api-doc.js): update paths if they scan src/sql / src/schema
• generate-schema-graph.js / schema-graph.dot: point at both schemas
• Deprecation note: new LibDashStore(pool) removed — constructor now takes a provider (breaking change; coordinate with dashback)

9. Cleanup

• Remove transitional this.pool / provider.pool usage from index.js
• Remove mssql from peerDependencies only if no runtime coupling remains (queries/schema stay mssql-specific, so likely keep it)
• Decide export for SCHEMA_SCRIPTS (currently re-exported from index.js from the moved schema file)

I'll check how auth is applied across the routes.

Viewed index.js:1-305 Searched for "app.(post|put|patch|delete|get|use|all)(" Viewed auth.js:1-132

The middleware skips auth on several path prefixes. Next I'll check which of those can write.

Searched for ".(post|put|patch|delete)(" Searched for "'/(api/)?(public|mcp|oauth|.well-known)"

Next: the telemetry public route, the /oauth/register handler, and the v1 router.

Viewed telemetry.js:1-118 Viewed mcp.js:360-460 Searched for "(router|app).(post|put|patch|delete|use|all)(|auth|token" Viewed index.js:1-27 Searched for "registerClient|export (async )?function" Viewed mcp.js:455-540 Searched for "db.\w+|verifyIdToken|allow|jwt.sign"

No. With no JWT, an attacker can still write to the database. The checks I did are static code review only, and I haven't run anything.

authMiddleware (auth.js) lets several path groups through without a token. Most of them can't write to the SQL database, but two can.

1. Open OAuth registration: POST /oauth/register

• This route is public, and it calls mcpDb.registerClient (mcp.js:368) to insert a client row.
• Anyone can create unlimited rows with no auth and no rate limit.
• client_name, redirect_uris, grant_types and scope all come straight from the request body.

2. Auth bypass: POST /oauth/authorize

• The middleware calls next() on this route even with no token (auth.js:70-79).
• The handler (mcp.js:455-503) takes an email from the body when there's no req.user.
• If that email is on the allowlist, it writes an auth code (createAuthCode) for that user and redirects with it. No password, Google token or JWT is checked.
• An attacker can chain this with the previous hole:
  1. Register a client with /oauth/register.
  2. Post to /oauth/authorize with the victim's allowlisted email and their own PKCE code_challenge.
  3. Exchange the code at /oauth/token, which I haven't read.
  4. Call /mcp as that user.
• That would be a full impersonation if the MCP tools can write. Please confirm this by testing against a dev instance.
• Also, new URL(redirect_uri) runs with no try/catch and no check that redirect_uri matches the registered URIs. A bad value throws, and a valid one is an open redirect that sends the code to the attacker.

Not writing to the SQL database

• /api/public/telemetry/crash (telemetry.js) is public and unauthenticated. It writes to a file on disk rather than SQL, but there's no size or rate limit beyond bodyLimit, so it can fill the disk.
• /v1 and /api/v1 skip the middleware, but the routes I saw are all GET. The ones with data use posAuthMiddleware, and /client, /pimage and /cimage have no auth.
• /api/auth/login verifies a Google ID token and checks the allowlist. If IMPLICIT_CREATE=true, any valid Google account gets added to the allowlist (login.js:107). Make sure that's off in production.

Suggested fixes

• Make /oauth/authorize POST require req.user, and remove the email fallback.
• Validate redirect_uri against the registered URIs for the client.
• Rate-limit /oauth/register, or gate it.

Want me to implement these? I haven't changed any code.