Harden raw-rule column guard + cover likePrefix + @import types (neutral review, PR #291)

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

Author: philcunliffe

hypaware-core/plugins-workspace/context-graph/src/contract-registry.js

@@ -92,8 +92,9 @@ export function createContractRegistry(opts = {}) {
         throw new TypeError(`registerContract: ${at} where is only valid with columns`)
       }
       if (hasSql && contract.rowFilter) {
+        const sql = /** @type {string} */ (rule.sql)
         for (const col of contract.rowFilter.columns) {
-          if (!rule.sql?.includes(col)) {
+          if (!rawSqlProjectsColumn(sql, col)) {
             throw new TypeError(`registerContract: ${at} raw sql must select rowFilter column '${col}'`)
           }
         }
@@ -170,3 +171,127 @@ export function createContractRegistry(opts = {}) {
 
   return { register, list }
 }
+
+/**
+ * True when a raw rule's SQL provably projects `col` in its top-level
+ * SELECT list, so the contract's rowFilter (which reads `row[col]`) has the
+ * column to test. A loose `sql.includes(col)` accepts false positives (a
+ * `WHERE attributes IS NOT NULL`, or a different column whose name merely
+ * contains `col`), so match the projection list only: take the identifiers
+ * between the first top-level SELECT and its FROM, accept `*` / `table.*`,
+ * and reduce each item to its output name (the alias after AS, or the column
+ * past a `table.` qualifier). Anything ambiguous (a computed expression with
+ * no alias) is treated as not-a-match, so the guard stays conservative and
+ * rejects registration when the column is not provably projected. This is a
+ * focused projection check, not a general SQL parser.
+ *
+ * @param {string} sql
+ * @param {string} col
+ * @returns {boolean}
+ */
+function rawSqlProjectsColumn(sql, col) {
+  const projection = selectProjection(sql)
+  if (projection === undefined) return false
+  for (const item of splitTopLevel(projection)) {
+    const name = projectionOutputName(item)
+    if (name === '*' || name === col) return true
+  }
+  return false
+}
+
+/**
+ * The text between the first top-level `SELECT` and its matching `FROM`
+ * (both matched as standalone, case-insensitive keywords at parenthesis
+ * depth 0, so a subquery's SELECT/FROM never leaks in). Undefined when the
+ * SQL has no top-level `SELECT ... FROM`.
+ *
+ * @param {string} sql
+ * @returns {string | undefined}
+ */
+function selectProjection(sql) {
+  const upper = sql.toUpperCase()
+  let depth = 0
+  let selectEnd = -1
+  for (let i = 0; i < sql.length; i++) {
+    const ch = sql[i]
+    if (ch === '(') depth++
+    else if (ch === ')') depth--
+    else if (depth === 0 && selectEnd === -1 && matchKeyword(upper, i, 'SELECT')) {
+      selectEnd = i + 'SELECT'.length
+      i = selectEnd - 1
+    } else if (depth === 0 && selectEnd !== -1 && matchKeyword(upper, i, 'FROM')) {
+      return sql.slice(selectEnd, i)
+    }
+  }
+  return undefined
+}
+
+/**
+ * True when `kw` sits at index `i` of `upper` as a whole word (its
+ * neighbours are not identifier characters), so `FROM` matches but
+ * `FROMAGE` or a `from_x` column does not.
+ *
+ * @param {string} upper
+ * @param {number} i
+ * @param {string} kw
+ * @returns {boolean}
+ */
+function matchKeyword(upper, i, kw) {
+  if (!upper.startsWith(kw, i)) return false
+  const boundary = (/** @type {string | undefined} */ c) => c === undefined || !/[A-Z0-9_]/.test(c)
+  return boundary(upper[i - 1]) && boundary(upper[i + kw.length])
+}
+
+/**
+ * Split on commas at parenthesis depth 0, so a `f(a, b)` projection item
+ * stays whole.
+ *
+ * @param {string} s
+ * @returns {string[]}
+ */
+function splitTopLevel(s) {
+  /** @type {string[]} */
+  const parts = []
+  let depth = 0
+  let start = 0
+  for (let i = 0; i < s.length; i++) {
+    const ch = s[i]
+    if (ch === '(') depth++
+    else if (ch === ')') depth--
+    else if (ch === ',' && depth === 0) {
+      parts.push(s.slice(start, i))
+      start = i + 1
+    }
+  }
+  parts.push(s.slice(start))
+  return parts
+}
+
+/**
+ * The output name a single projection item exposes on the result row: the
+ * alias after `AS`, `*` for a wildcard (`*` or `table.*`), or the column
+ * past a `table.` qualifier. A bare computed expression has no derivable
+ * column name and returns its trimmed text, which will not match a plain
+ * column name, keeping the guard conservative.
+ *
+ * @param {string} item
+ * @returns {string}
+ */
+function projectionOutputName(item) {
+  let s = item.trim()
+  if (s.length === 0) return ''
+  const asMatch = /\s+AS\s+("?[A-Za-z0-9_]+"?)\s*$/i.exec(s)
+  if (asMatch) return stripQuotes(asMatch[1])
+  if (s === '*' || s.endsWith('.*')) return '*'
+  const dot = s.lastIndexOf('.')
+  if (dot !== -1) s = s.slice(dot + 1)
+  return stripQuotes(s)
+}
+
+/**
+ * @param {string} s
+ * @returns {string}
+ */
+function stripQuotes(s) {
+  return s.startsWith('"') && s.endsWith('"') && s.length >= 2 ? s.slice(1, -1) : s
+}

hypaware-core/plugins-workspace/context-graph/src/project.js

@@ -14,7 +14,7 @@ import {
 /**
  * @import { HypAwareV2Config, QueryRegistry } from '../../../../hypaware-plugin-kernel-types.js'
  * @import { ExtendedQueryStorageService } from '../../../../src/core/cache/types.js'
- * @import { Contract, GraphRow } from './types.js'
+ * @import { Contract, ContractRule, GraphRow, RulePredicate } from './types.js'
  */
 
 /**
@@ -52,7 +52,7 @@ export async function projectGraph({ query, storage, contracts, config, dryRun =
        * one-query-per-rule order.
        *
        * @param {Record<string, unknown>} row
-       * @param {import('./types.js').ContractRule[]} rules
+       * @param {ContractRule[]} rules
        */
       const applyRules = (row, rules) => {
         for (const rule of rules) {
@@ -107,7 +107,7 @@ export async function projectGraph({ query, storage, contracts, config, dryRun =
         // rule pairs sharing a query (a surface's node + edge rule) cost one
         // scan. Their SQL already selects the rowFilter's columns (registry
         // enforced), so the filter applies here too.
-        /** @type {Map<string, import('./types.js').ContractRule[]>} */
+        /** @type {Map<string, ContractRule[]>} */
         const bySql = new Map()
         for (const rule of raw) {
           const sql = /** @type {string} */ (rule.sql)
@@ -213,12 +213,12 @@ async function dedupExisting(rows, idCol, dataset, query, storage, config) {
  * semantics: every clause must hold, and a null or absent column never
  * matches (`WHERE col = 'x'` skips null rows; so does this).
  *
- * @param {import('./types.js').RulePredicate} where
+ * @param {RulePredicate} where
  * @param {Record<string, unknown>} row
  * @returns {boolean}
  * @ref LLP 0096#decision [implements]: eq/in/likePrefix only, AND-composed, null never matches
  */
-function matchesPredicate(where, row) {
+export function matchesPredicate(where, row) {
   if (where.eq) {
     for (const [col, value] of Object.entries(where.eq)) {
       if (row[col] !== value) return false
@@ -244,7 +244,7 @@ function matchesPredicate(where, row) {
  * no rule's `columns` lists them (a rule may filter on a column `toRow`
  * never touches).
  *
- * @param {import('./types.js').RulePredicate | undefined} where
+ * @param {RulePredicate | undefined} where
  * @returns {string[]}
  */
 function predicateColumns(where) {

test/plugins/context-graph-contract.test.js

@@ -161,6 +161,34 @@ test('contract registry validates rowFilter, and raw sql must select its columns
   const raw = { kind: 'node', type: 'T', sql: 'SELECT a FROM x', toRow: () => null }
   assert.throws(() => reg.register(sampleContract({ name: 'y', rules: [raw], rowFilter: filter })), /raw sql must select rowFilter column 'attributes'/)
   reg.register(sampleContract({ name: 'z', rules: [{ ...raw, sql: 'SELECT attributes, a FROM x' }], rowFilter: filter }))
+
+  // The guard checks the SELECT projection list, not a loose substring: the
+  // column named in a WHERE clause, or a different column whose name merely
+  // contains it, is not projected and must be rejected.
+  const notProjected = [
+    'SELECT a FROM x WHERE attributes IS NOT NULL',
+    'SELECT other_attributes FROM x',
+    'SELECT a FROM x WHERE b IN (SELECT attributes FROM y)',
+  ]
+  for (const sql of notProjected) {
+    assert.throws(
+      () => reg.register(sampleContract({ name: 'n', rules: [{ ...raw, sql }], rowFilter: filter })),
+      /raw sql must select rowFilter column 'attributes'/,
+      sql
+    )
+  }
+
+  // Projection forms that do provably carry the column are accepted: a
+  // qualified reference, an alias, and a wildcard.
+  const projected = [
+    'SELECT x.attributes, a FROM x',
+    'SELECT foo AS attributes, a FROM x',
+    'SELECT * FROM x',
+    'SELECT x.* FROM x',
+  ]
+  projected.forEach((sql, i) => {
+    reg.register(sampleContract({ name: `p${i}`, rules: [{ ...raw, sql }], rowFilter: filter }))
+  })
 })
 
 test('contract registry rejects a duplicate (plugin, name)', () => {

test/plugins/context-graph-project.test.js

@@ -3,7 +3,7 @@
 import assert from 'node:assert/strict'
 import test from 'node:test'
 
-import { firstSeenTime, mergeRow } from '../../hypaware-core/plugins-workspace/context-graph/src/project.js'
+import { firstSeenTime, matchesPredicate, mergeRow } from '../../hypaware-core/plugins-workspace/context-graph/src/project.js'
 
 /**
  * @param {Partial<Record<string, unknown>>} overrides
@@ -93,6 +93,23 @@ test('mergeRow tolerates rows with unparseable or missing first_seen', () => {
   assert.equal(ba.first_seen, '2026-06-01T00:00:00.000Z')
 })
 
+// The equivalence e2e (context-graph-project-e2e.test.js) claims likePrefix
+// coverage, but the ai-gateway prefix surfaces run as raw SQL, so its JS
+// `likePrefix` branch is never actually evaluated there. Exercise it directly.
+// @ref LLP 0096#decision [tests]: likePrefix matches a string prefix; null and non-string columns never match
+test('matchesPredicate likePrefix keeps only matching-prefix string rows', () => {
+  const where = { likePrefix: { content_text: 'Base directory for this skill: ' } }
+  const match = { content_text: 'Base directory for this skill: /repo/skills/foo' }
+  const nonMatch = { content_text: 'a different message' }
+  const nullValue = { content_text: null }
+  const nonString = { content_text: 42 }
+
+  assert.equal(matchesPredicate(where, match), true, 'matching prefix is kept')
+  assert.equal(matchesPredicate(where, nonMatch), false, 'non-matching string is dropped')
+  assert.equal(matchesPredicate(where, nullValue), false, 'null never matches')
+  assert.equal(matchesPredicate(where, nonString), false, 'non-string never matches')
+})
+
 test('firstSeenTime normalizes strings, Dates, and epoch numbers', () => {
   const t = Date.UTC(2026, 5, 1)
   assert.equal(firstSeenTime('2026-06-01T00:00:00.000Z'), t)