|
178 | 178 | "# require parsing the Cypher AST, which is out of scope for a demo safety guard.\n", |
179 | 179 | "# In production code, use server-side row-level security instead of client regex.\n", |
180 | 180 | "# Scope guards. Presence is not enough: the session predicate has to sit\n", |
181 | | - "# inside the WHERE clause it claims to restrict, and it must be real syntax\n", |
182 | | - "# rather than text. Three ways a query slipped past an earlier version:\n", |
| 181 | + "# inside the WHERE clause it claims to restrict, and it must be live syntax\n", |
| 182 | + "# rather than text the engine ignores. Four ways a query slipped past an\n", |
| 183 | + "# earlier version:\n", |
183 | 184 | "#\n", |
184 | 185 | "# WITH n, n.session = $sess AS unused WHERE true equality before the WHERE\n", |
185 | 186 | "# WHERE true RETURN n, n.session = $sess AS scoped equality after it, in RETURN\n", |
186 | 187 | "# RETURN n AS `WHERE n.session = $sess` the whole thing quoted\n", |
| 188 | + "# WHERE true /* n.session = $sess */ commented out, so ignored\n", |
187 | 189 | "#\n", |
188 | | - "# So quoted spans are blanked first, and the search is bounded to the WHERE\n", |
189 | | - "# clause: from the keyword to whichever clause opens next. Every pattern below\n", |
190 | | - "# excludes its own delimiters, so none backtracks across a long query.\n", |
191 | | - "_QUOTED_RE = re.compile(r\"'[^']*'|\\\"[^\\\"]*\\\"|`[^`]*`\")\n", |
| 190 | + "# Quoted spans and comments are therefore blanked in one left-to-right pass:\n", |
| 191 | + "# whichever opens first wins, so `//` inside a string stays part of the string\n", |
| 192 | + "# and a quote inside a comment does not open one. The search is then bounded\n", |
| 193 | + "# to the WHERE clause, from the keyword to whichever clause opens next. Every\n", |
| 194 | + "# pattern below excludes its own delimiters, so none backtracks across a long\n", |
| 195 | + "# query.\n", |
| 196 | + "_MASKED_RE = re.compile(\n", |
| 197 | + " r\"'[^']*'|\\\"[^\\\"]*\\\"|`[^`]*`|/\\*.*?\\*/|//[^\\n]*\",\n", |
| 198 | + " re.DOTALL,\n", |
| 199 | + ")\n", |
192 | 200 | "_WHERE_RE = re.compile(r\"\\bWHERE\\b\", re.IGNORECASE)\n", |
193 | 201 | "_CLAUSE_START_RE = re.compile(\n", |
194 | 202 | " r\"\\b(RETURN|WITH|MATCH|OPTIONAL|ORDER\\s+BY|SKIP|LIMIT|UNION|CALL|MERGE\"\n", |
|
202 | 210 | ")\n", |
203 | 211 | "\n", |
204 | 212 | "\n", |
| 213 | + "def _blank(match: re.Match) -> str:\n", |
| 214 | + " \"\"\"Replace a quoted span with empty quotes and a comment with a space.\"\"\"\n", |
| 215 | + " text = match.group(0)\n", |
| 216 | + " return \" \" if text.startswith((\"/*\", \"//\")) else \"''\"\n", |
| 217 | + "\n", |
| 218 | + "\n", |
205 | 219 | "def _is_session_scoped(query: str) -> bool:\n", |
206 | 220 | " \"\"\"Whether the query restricts itself to this session.\n", |
207 | 221 | "\n", |
|
210 | 224 | " $sess})` still slips through. Production code puts this on the server as\n", |
211 | 225 | " row-level security rather than in a client regex.\n", |
212 | 226 | " \"\"\"\n", |
213 | | - " bare = _QUOTED_RE.sub(\"''\", query)\n", |
| 227 | + " bare = _MASKED_RE.sub(_blank, query)\n", |
214 | 228 | " for where in _WHERE_RE.finditer(bare):\n", |
215 | 229 | " nxt = _CLAUSE_START_RE.search(bare, where.end())\n", |
216 | 230 | " clause = bare[where.end(): nxt.start() if nxt else len(bare)]\n", |
|
318 | 332 | "### The guard, checked\n", |
319 | 333 | "\n", |
320 | 334 | "`query_facts` runs whatever Cypher the model writes, so the scope check is the\n", |
321 | | - "only thing keeping one session out of another's facts. Three queries below read\n", |
| 335 | + "only thing keeping one session out of another's facts. Five queries below read\n", |
322 | 336 | "every session and have to be refused: the predicate placed in `RETURN` instead\n", |
323 | | - "of `WHERE`, the same predicate hidden inside a backtick-quoted alias, and one\n", |
324 | | - "sitting in a string literal. This runs on every execution, so a loosened regex\n", |
325 | | - "fails here rather than in front of a user.\n" |
| 337 | + "of `WHERE`, the same predicate hidden inside a backtick-quoted alias, one\n", |
| 338 | + "sitting in a string literal, and one commented out in each of Cypher's two\n", |
| 339 | + "comment forms. This runs on every execution, so a loosened regex fails here\n", |
| 340 | + "rather than in front of a user.\n" |
326 | 341 | ] |
327 | 342 | }, |
328 | 343 | { |
|
336 | 351 | " \"MATCH (n {session: $sess}) RETURN n\",\n", |
337 | 352 | " \"MATCH (a)-[r]->(b {session: $sess}) RETURN a, b\",\n", |
338 | 353 | " \"MATCH (n) WHERE n.session = $sess RETURN n ORDER BY n.name LIMIT 5\",\n", |
| 354 | + " # A comment must not swallow the predicate next to it.\n", |
| 355 | + " \"MATCH (n) // pick this session\\n WHERE n.session = $sess RETURN n\",\n", |
| 356 | + " \"MATCH (n) WHERE n.session = $sess /* scoped */ RETURN n\",\n", |
| 357 | + " # '//' inside a string opens no comment.\n", |
| 358 | + " \"MATCH (n) WHERE n.url = 'http://x' AND n.session = $sess RETURN n\",\n", |
339 | 359 | "]\n", |
340 | 360 | "unscoped = [\n", |
341 | 361 | " \"MATCH (n) RETURN n\",\n", |
|
347 | 367 | " \"MATCH (n) RETURN n AS `WHERE n.session = $sess`\",\n", |
348 | 368 | " # A string literal is data; it restricts nothing.\n", |
349 | 369 | " \"WITH '.session = $sess' AS note MATCH (n) WHERE true RETURN n\",\n", |
| 370 | + " # The engine ignores both comment forms, so neither restricts anything.\n", |
| 371 | + " \"MATCH (n) WHERE true /* n.session = $sess */ RETURN n\",\n", |
| 372 | + " \"MATCH (n) WHERE true // n.session = $sess\\n RETURN n\",\n", |
350 | 373 | "]\n", |
351 | 374 | "\n", |
352 | 375 | "wrong = [q for q in scoped if not _is_session_scoped(q)]\n", |
|
0 commit comments