Skip to content

Restrict route keyword matching to building field#33

Merged
krotname merged 1 commit into
masterfrom
codex/fix-vulnerability-in-route-keyword-handling
Jun 25, 2026
Merged

Restrict route keyword matching to building field#33
krotname merged 1 commit into
masterfrom
codex/fix-vulnerability-in-route-keyword-handling

Conversation

@krotname

Copy link
Copy Markdown
Owner

Motivation

  • Route keyword filters were being evaluated against the full raw Telegram message text, which allowed a whitelisted sender to inject another route's keyword in free-form fields and cause PII disclosure to unintended targets.
  • Matching should use a trusted, parsed field (the building value) instead of attacker-controlled free text to enforce route isolation.

Description

  • Route selection now matches keywords against the parsed building field instead of message.text by passing parsed_request.fields["building"] to RouteRule.matches in src/telegram_resender/service.py.
  • Added a regression test test_service_matches_route_keywords_only_against_building_field in tests/unit/test_service.py that asserts a keyword injected into a comment does not cause an extra route to be selected.
  • Ran code formatting/linters which reformatted service.py as needed.

Testing

  • Ran formatter/linter checks: python -m ruff format src/telegram_resender/service.py tests/unit/test_service.py and python -m ruff check src/telegram_resender/service.py tests/unit/test_service.py, which passed.
  • Ran targeted unit tests: python -m pytest -o addopts='' tests/unit/test_service.py tests/unit/test_routes.py, resulting in 8 passed for those files.
  • Attempted full test run python -m pytest -o addopts='' which failed with async test errors because pytest-asyncio is not installed in the environment, causing many async tests to fail; attempted pip install -e '.[dev]' to add dev deps but network access to package index failed (403) so dev dependencies were not installed.

Codex Task

@krotname krotname merged commit 86dbc90 into master Jun 25, 2026
10 checks passed
@krotname krotname deleted the codex/fix-vulnerability-in-route-keyword-handling branch June 25, 2026 05:28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6da546801d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

route
for route in self._routes
if route.matches(username=message.user.username, text=message.text)
if route.matches(username=message.user.username, text=parsed_request.fields["building"])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep blacklist checks outside building

For routes that use keywords_none as a blocklist for request-level words (for example the documented "cancel"/"отмена" route examples), passing only parsed_request.fields["building"] means RouteRule.matches() now checks both allow and deny keywords only against the building value. A complete request like Building: Tower A ... Comment: cancel will still match and be forwarded, whereas before the negative keyword suppressed it; this should separate building-only positive route selection from request-wide negative filtering if blacklist keywords are meant to block canceled or disallowed requests outside the building field.

Useful? React with 👍 / 👎.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant