fix: table_name validation accepts a trailing newline#317
Open
Andrew Chen (chuenchen309) wants to merge 1 commit into
Open
fix: table_name validation accepts a trailing newline#317Andrew Chen (chuenchen309) wants to merge 1 commit into
Andrew Chen (chuenchen309) wants to merge 1 commit into
Conversation
… vs re.fullmatch) `re.match(r"^\w+$", table_name)` accepts a table_name ending in a single trailing newline, e.g. "chat_history\n": without re.MULTILINE, `$` matches either at the very end of the string or immediately before a trailing "\n". The intent (per the error message: "must contain only alphanumeric characters and underscores") is a full-string match. Fix: use re.fullmatch(r"\w+", table_name), which requires the entire string to match with no such trailing-newline exception. table_name is later interpolated into DDL via psycopg's sql.Identifier, so this mostly affects error surfacing/consistency rather than a raw injection (Identifier quoting still applies), but it's still not the validation the code claims to perform (CWE-625: permissive regex). Testing: added test_table_name_rejects_trailing_newline -- __init__ validates table_name before ever touching the connection, so a dummy object() is enough to reach the check without a real DB. Confirmed red->green (reverting only chat_message_histories.py makes the new test fail with "DID NOT RAISE ValueError"; reapplying passes). ruff check/format and mypy clean on the changed files. The two existing DB-backed tests in this file require a live Postgres connection unavailable in this environment; my change only affects validation that runs before any connection is used. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PostgresChatMessageHistory.__init__validatestable_namewithre.match(r"^\w+$", table_name). Withoutre.MULTILINE,$matches either at the very end of the string or immediately before a single trailing"\n". So a table name like"chat_history\n"passes this check, even though the stated intent (per the raised error message) is "Table name must contain only alphanumeric characters and underscores."Fix
Use
re.fullmatch(r"\w+", table_name)instead, which requires the entire string to match with no trailing-newline exception.table_nameis later interpolated into DDL via psycopg'ssql.Identifier(which still applies its own quoting), so this is primarily about the validation actually doing what its error message claims, not a raw SQL-injection vector — but a permissive validation regex like this is still worth tightening (CWE-625).Testing
test_table_name_rejects_trailing_newline:__init__validatestable_namebefore ever touching the connection, so a dummyobject()forsync_connectionis enough to reach the check without needing a real DB.chat_message_histories.pymakes the new test fail withDID NOT RAISE ValueError; reapplying the fix passes.ruff check/ruff format --diffandmypyclean on both changed files.test_sync_chat_history,test_async_chat_history) require a live Postgres connection unavailable in my sandbox; my change only touches validation that runs before any connection is used, so it doesn't affect their logic.AI-Generated disclosure
Found via an AI-assisted code review pass (Claude Code) over
langchain_postgres/. I personally reproduced the permissive-regex behavior, verified the fix, and ran the relevant test file plus lint/mypy before submitting.