Skip to content

fix(api): reject trailing newlines in time_duration / parse_time_duration (#39730) - #39731

Open
Harsh23Kashyap wants to merge 1 commit into
langgenius:mainfrom
Harsh23Kashyap:fix/39730-time-duration-trailing-newline
Open

fix(api): reject trailing newlines in time_duration / parse_time_duration (#39730)#39731
Harsh23Kashyap wants to merge 1 commit into
langgenius:mainfrom
Harsh23Kashyap:fix/39730-time-duration-trailing-newline

Conversation

@Harsh23Kashyap

Copy link
Copy Markdown
Contributor

Fixes #39730.

Summary

Two time-input validators in api/libs/ used re.match with a $ anchor. In Python, $ matches at end-of-string OR just before a trailing newline, so values like "7d\n", "30m\n", and "4h\n" slipped through. Replaced re.match with re.fullmatch in both, matching the pattern established by the email fix in #39320, the password fix in #39548, and the alphanumeric fix in #39666 (merged 2026-07-28).

Sites

  • api/libs/custom_inputs.py:6time_duration. Field validator on WorkflowRunListQuery.time_range (api/controllers/console/app/workflow_run.py:101), the user-facing filter on the workflow-runs list endpoint.
  • api/libs/time_parser.py:7parse_time_duration. Same regex duplicated; used by workflow scheduling and similar surfaces.

Why it matters

time_duration is the field validator on a real user-facing endpoint. A value with a trailing \n is silently forwarded to the SQL WHERE clause. The query still works (parameter binding is whitespace-tolerant), but downstream consumers that echo the value — log lines, error messages, response payloads — get the newline embedded. Same log-injection / header-injection surface that the email and alphanumeric fixes closed.

Changes

  • api/libs/custom_inputs.py:6re.matchre.fullmatch with a 4-line comment matching the style of the email and alphanumeric fixes.
  • api/libs/time_parser.py:7 — same fix, same comment style.
  • api/tests/unit_tests/libs/test_custom_inputs.py — 5 new tests in TestTimeDuration: trailing newline, trailing CR, trailing CRLF, leading newline, embedded whitespace.
  • api/tests/unit_tests/libs/test_time_parser.py — 6 new tests in TestParseTimeDuration: trailing newline, trailing CR, trailing CRLF, leading newline, embedded whitespace.

Verification

cd api
.venv/bin/python -m pytest tests/unit_tests/libs/test_custom_inputs.py tests/unit_tests/libs/test_time_parser.py -v
# 32 passed in 0.56s  (21 existing + 11 new)

.venv/bin/python -m ruff check libs/custom_inputs.py libs/time_parser.py tests/unit_tests/libs/test_custom_inputs.py tests/unit_tests/libs/test_time_parser.py
# All checks passed!

.venv/bin/python -m ruff format --check libs/custom_inputs.py libs/time_parser.py tests/unit_tests/libs/test_custom_inputs.py tests/unit_tests/libs/test_time_parser.py
# 4 files already formatted

Broader sweep to make sure nothing else regressed:

cd api
.venv/bin/python -m pytest tests/unit_tests/libs/ -q
# 612 passed in 13.4s

Scope

  • Two source lines (one per file), each with a 4-line comment.
  • Two new test sets (one per file).
  • No API change, no schema change, no migration. Existing rows unaffected.

Out of scope

A scan of api/ for other re.match patterns with $ anchors turned up a few more sites in path/URL validators that already have separate guards, and a couple in tool internals where the input is form-supplied. This PR is scoped to the two highest-impact, user-facing sites; happy to file a follow-up sweep if maintainers want it.

…tion (langgenius#39730)

Same bug class as the email validator fix in langgenius#39320, the password
validator fix in langgenius#39548, and the alphanumeric validator fix in langgenius#39666
(merged 2026-07-28). Two more sites in api/libs/ used re.match with a
'$' anchor; in Python '$' matches at end-of-string OR just before a
trailing newline, so values like '7d\n', '30m\n', and '4h\n' slipped
through.

time_duration in api/libs/custom_inputs.py:6 is the field validator
on WorkflowRunListQuery.time_range (api/controllers/console/app/
workflow_run.py:101). A time_range ending in '\n' is silently
forwarded to the SQL WHERE clause — the same log-injection /
header-injection surface that the email and alphanumeric fixes closed.

parse_time_duration in api/libs/time_parser.py:7 is the same regex
duplicated in a second file; used by the workflow scheduler and
similar surfaces.

Replace re.match with re.fullmatch in both, and add trailing-newline,
trailing-CR, trailing-CRLF, leading-newline, and embedded-whitespace
regression tests in both test files.

The remaining re.match-with-trailing-newline sites in api/libs/ are
either in path/URL validators that already have separate guards or
in tool internals where the input is form-supplied; not a
user-facing security surface. Filed the time_duration / parse_time
pair here, and noted the rest for a follow-up sweep if maintainers
want it.
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 55.75% 55.74% -0.01%
Strict coverage 55.26% 55.25% -0.01%
Typed symbols 36,348 36,348 0
Untyped symbols 29,104 29,114 +10
Modules 3087 3087 0

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

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

time_duration / parse_time_duration accept trailing newline (sibling of #39234, #39548, #39666)

1 participant