Skip to content

KB: review merged PR #311 - fix(security): shape-constrain lexical match text before SQL interpolation (ISS-006 hardening) #312

Description

@github-actions

A merged PR may require knowledge-base updates. Review the change and update library/ as needed.

Source PR

  • PR: #311 - fix(security): shape-constrain lexical match text before SQL interpolation (ISS-006 hardening)
  • Author: @thenotoriousllama
  • Merged into: main
  • Files changed: 11

PR description

Security hardening follow-up to #307 (Aikido "2 new HIGH" analysis)

security-worker-bee audit of the ISS-006 tokenized lexical fallback that landed in #307. #307 merged mid-audit, so the remediation lands here as a follow-up off main instead of on the (now deleted) PR branch.

Independent analysis of the Aikido flag

The scanner's taint pattern (request body -> string-concatenated SQL) keys on two spots in src/daemon/runtime/memories/recall.ts:

  1. buildLexicalMatchSql(colSql, term) — free-text term interpolation. Injection itself is closed: every token and the whole phrase route through sqlLike (quote doubling, \ % _ escaping, C0/DEL stripping) inside a plain '...' literal, and identifiers ride sqlIdent. What was real: the recall route schema has no max on query (z.string().min(1) — hooks pass whole prompts), and the tokenized predicate interpolates the query up to 9x per matched column (whole phrase + 8 conjuncts; 27x on the three-column hive-graph arm). A multi-megabyte query became an unbounded statement-size amplification — the resource-exhaustion half of the finding.
  2. colSql as a pre-built fragment parameter. All call sites are compile-time constants built from sqlIdent parts, but the function accepted any string — the exact seam a future dynamic call site could turn into an injection.

Remediation (this PR)

  • Pre-interpolation shaping, mirroring the feat: injected-token metering, live KPI, real ROI trend, partial net #298 precedent (validate/shape-constrain before interpolation): control chars (C0+DEL) stripped, per-token cap MAX_LEXICAL_TOKEN_LENGTH = 128, whole-phrase cap MAX_LEXICAL_PHRASE_LENGTH = 1024. Truncation of an ILIKE substring pattern only ever widens the admitted set, so recall is never silently narrowed; any query under the caps emits byte-identical SQL (regression-asserted).
  • colSql contract documented as a security invariant + a tamper canary: a ;, --, or /* in the fragment throws before any SQL is built, so a dyn

Files touched

  • .claude-plugin/marketplace.json (+2/-2)
  • .claude-plugin/plugin.json (+1/-1)
  • CHANGELOG.md (+4/-0)
  • harnesses/claude-code/.claude-plugin/plugin.json (+1/-1)
  • harnesses/codex/package.json (+1/-1)
  • harnesses/openclaw/openclaw.plugin.json (+1/-1)
  • harnesses/openclaw/package.json (+1/-1)
  • package-lock.json (+2/-2)
  • package.json (+1/-1)
  • src/daemon/runtime/memories/recall.ts (+52/-5)
  • tests/daemon/runtime/memories/iss-006-search-list-corpus-parity.test.ts (+53/-0)

Auto-generated by .github/workflows/kb-issue-on-merge.yaml on merge.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    knowledgeKB update prompted by a merged PR

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions