Files
accounted/extensions/general
Jakob WennbergandClaude Opus 4.7 6629964780 fix(mcp): unblock query_journal text search across line + entry descriptions (#605)
* fix(mcp): unblock query_journal text search across line+entry descriptions

`gnubok_query_journal` issued a single PostgREST `.or()` filter mixing a
base-table column (line_description on journal_entry_lines) with a column
on the embedded inner-joined resource (journal_entries.description).
PostgREST's flat comma OR syntax cannot span base and embedded resources
and returned 400 "failed to parse logic tree" on every free-text search,
so the agent could not look up history (e.g. "har vi bokfört Google
innan?") via the MCP tool.

Refactor: pull the common filter chain into a `buildBaseQuery()` helper
and issue two parallel `.ilike()` queries — one on `line_description`,
one on `journal_entries.description` — then merge by line id, re-sort,
and slice to `limit`. Same pattern as
`lib/invoices/duplicate-payment-candidates.ts`, with the same rationale
(LIKE-DSL injection risk + cross-embed unsupported).

Drops the obsolete comma-stripping pre-processing on the search term
(only needed because the value was being injected into PostgREST's OR
DSL); LIKE wildcard escaping (`%` and `_`) is preserved.

Adds four tests covering: merge across both legs, dedup of cross-leg
overlap, correct columns + escaped pattern sent to `.ilike`, and
LIKE-wildcard escape end-to-end. Existing amount-filter / accounts-cap /
truncation tests unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(mcp): query_journal PR review — honest truncation, wider leg, error sanitisation

Address PR #605 review (Greptile + compliance swarm).

- truncated math: previously dbMatched = byLine.count + byEntry.count
  inflated the counter every time a row matched both legs (e.g. an entry
  whose line description AND header description both contain "Google").
  total_lines was off by up to 2× and truncated was forced true even when
  every distinct match was already returned, sending the agent into
  unnecessary narrow-and-retry loops. dbMatched is now merged.size — the
  honest distinct-row count among what we fetched. A new legCapHit signal
  drives truncated when either leg's fetch window filled, so we never lie
  the other direction either.

- per-leg fetch widened to 2 × limit. Previously each leg was capped at
  `limit` independently, so when one column was much more selective than
  the other (e.g. 150 line matches vs 5 entry matches), the merge could
  drop globally-ranked rows from the chronologically newer tail. The
  final slice still caps at `limit`; the wider per-leg window just gives
  the merge a better tail to choose from.

- text input length cap (200 chars). Defence-in-depth against pathological
  inputs even though .ilike() parameterises the value (compliance A.8.28).

- error sanitisation in the text-search path. Raw PostgREST messages can
  surface internal schema details (table names, constraint names) to the
  caller. We now log details server-side via the existing module logger
  and throw a generic message (compliance V16).

- comma-stripping rationale documented inline so the next reviewer
  doesn't re-add it. The previous defence was needed because the value
  was injected into PostgREST's OR DSL where `,` is the separator; the
  parameterised .ilike() path treats `,` as a literal and stripping it
  would mangle real-comma searches (compliance V1.2.5).

Adds 4 regression tests: overlap-doesn't-falsely-truncate, leg-cap-hit
flags truncated, text>200 rejected, raw schema name never leaked. All
14 query_journal tests pass; 266/266 MCP suite green; build clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(mcp): query_journal PR review round 2 — schema guards, leg cap, audit log

Address compliance swarm findings on commit 3dfab46c.

- text input: add `maxLength: 200` to the JSON Schema so the limit is
  enforced at the boundary by the MCP runtime, not only inside execute().
  Inline guard kept as belt-and-suspenders (ASVS V2.2, ISO 27001 A.8.28).
- limit input: add `minimum: 1, maximum: 500` to the JSON Schema (matches
  the existing Math.min cap inside execute()) so the bound is visible to
  agents and enforced declaratively.
- legLimit hard cap: `Math.min(limit * 2, 500)`. Previously legLimit
  scaled with limit unbounded, so a maxed-out limit triggered 2×1000-row
  fetches. Cap is independent of the caller-supplied limit (ASVS V2.3).
- audit log: include `userId` alongside `companyId` in both log.warn
  payloads so failed-query events can be correlated to the actor for
  incident investigation (ASVS V16, SOC 2 CC7.2).
- new test: assert both parallel ilike legs in the text-search path issue
  `.eq('journal_entries.company_id', companyId)` — defence-in-depth
  against a future refactor accidentally dropping tenant scoping from
  one leg (SOC 2 CC6.1).

Not addressed (with rationale):

- ASVS V1.2.5 / SOC 2 CC7.2 "log.warn body contains raw PostgREST error
  text". The swedish-accounting-compliance reviewer explicitly cleared
  this on the same commit ("the schema-leak is plugged in both paths").
  The thrown error is generic; raw detail lives only in internal
  structured logs (Sentry, etc.) where it is needed for incident triage.
  Same pattern is used by 20+ other tools in this file; scrubbing only
  this one is inconsistent. Defer to a project-wide logger sanitiser.

- Swedish reviewer's "reversed mixed with posted in status=all": false
  positive — `status` IS surfaced per line in the response shape (see
  the lines.map projection in server.ts).

- Swedish reviewer's "debit XOR credit not enforced": insert-time
  concern, not a read-tool concern. Out of scope.

Tests: 15 query_journal cases (4 new this round); 267 MCP suite; build
clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-29 14:21:46 +02:00
..
2026-05-13 00:26:04 +02:00