fix(enhance): strip trailing slashes without a backtracking regex (CodeQL #4) - #128
Conversation
…deQL js/polynomial-redos #4) CodeQL alert #4 flags `/\/+$/` applied to a caller-configured baseUrl in the EnhanceAPI constructor: on inputs shaped `////...x` the pattern re-scans quadratically. src/url-util.ts already carries the linear stripTrailingSlashes loop written to close this exact finding, and custody/automations/realtime/runtime already use it — enhance.ts was the last caller of the regex. Behaviour is unchanged for every input; covered by a new test asserting a `///` baseUrl still produces a single-slash /v1/enhance URL. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Skipping PR review because a bot author is detected. If you want to trigger CodeAnt AI, comment |
Reviewer's GuideEnhanceAPI now uses the repository’s linear stripTrailingSlashes helper instead of an end-anchored backtracking regex, eliminating the CodeQL polynomial- ReDoS finding while preserving URL behavior. A regression test verifies correctly formed enhancement request URLs for caller-configured base URLs with multiple trailing slashes. Sequence diagram for normalized EnhanceAPI request URLssequenceDiagram
participant Client
participant EnhanceAPI
participant stripTrailingSlashes
participant Fetch
Client->>EnhanceAPI: new EnhanceAPI(client)
EnhanceAPI->>Client: getConnectionInfo()
Client-->>EnhanceAPI: baseUrl with trailing slashes
EnhanceAPI->>stripTrailingSlashes: stripTrailingSlashes(info.baseUrl)
stripTrailingSlashes-->>EnhanceAPI: normalized baseUrl
Client->>EnhanceAPI: enhance(model)
EnhanceAPI->>Fetch: POST normalized baseUrl/v1/enhance?model=espcn
Fetch-->>EnhanceAPI: response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
| Filename | Overview |
|---|---|
| src/enhance.ts | Reuses the established linear trailing-slash helper with behavior equivalent to the replaced regex. |
| src/tests/enhance.test.ts | Adds deterministic coverage confirming normalized base URLs produce the expected enhancement endpoint. |
Reviews (1): Last reviewed commit: "fix(enhance): strip trailing slashes wit..." | Re-trigger Greptile
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This is a small, behavior-preserving change that reuses existing linear URL normalization and adds focused regression coverage. Human review remains appropriate because it is explicitly a CodeQL polynomial-ReDoS security remediation and modifies files owned by the core team rather than the stated author. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
yakimoto
left a comment
There was a problem hiding this comment.
CODEOWNER approval for the exemption proof
What
Replaces the trailing-slash regex in the
EnhanceAPIconstructor with the repo's existinglinear helper:
plus the matching
import { stripTrailingSlashes } from './url-util';. That is the entireproduction change — two lines in
src/enhance.ts.Why
CodeQL alert #4 (
js/polynomial-redos) fires onsrc/enhance.ts:57:https://github.com/wave-av/sdk/security/code-scanning/4
The pattern
/\/+$/is anchored at the end, so on an input shaped////…xthe engine restartsthe
\/+run at each successive slash position and re-scans to the end each time — quadratic inthe length of the slash run. The
baseUrlhere arrives fromclient.getConnectionInfo(), i.e.caller-supplied SDK configuration rather than attacker-controlled wire data, so the practical
exposure is small. It is still worth removing at the source: it is the last live instance of a
shape this repo has already decided against, and leaving one behind keeps the alert open and
trains the next contributor to copy it.
Precedent — this is not a new pattern
src/url-util.tsalready exportsstripTrailingSlashes(input: string): string, a plaincharCodeAtloop that was written specifically to replace this regex, and its docblock namesthe same CodeQL rule. It is already the way four other modules do this:
src/custody.ts:51—this.baseUrl = stripTrailingSlashes(opts.baseUrl)src/automations.ts:25—this.endpoint = stripTrailingSlashes(opts.endpoint)src/realtime.ts:47,127—this.wsBase = stripTrailingSlashes(...)src/runtime.ts:69—this.baseUrl = stripTrailingSlashes(opts.baseUrl)enhance.tswas simply the one caller that never got migrated. A grep ofsrc/forreplace(/\/+$/now returns only the prose reference inside theurl-util.tsdocblock — thereare no remaining code sites.
Behaviour is identical for every input, including the edge cases already pinned by
src/__tests__/url-util.test.ts(''→'','////'→'', no-trailing-slash input returnedby identity). Only the scanning cost changes, from backtracking to linear.
Test evidence
One new case added to the existing
src/__tests__/enhance.test.ts, mirroring how the file'sother tests build a mock
WaveClientand stubglobal.fetch: a client configured withbaseUrl: "https://api.wave.online///"must still requesthttps://api.wave.online/v1/enhance?model=espcn.This is a real guard, not a tautology —
new URL()does not collapse duplicate path slashes, sowithout the strip the SDK would call
https://api.wave.online////v1/enhance.(The
::error::tag sdk-v9.9.9 …lines in the test output are asserted-on stderr from therelease-tag guard test, not failures.)
dist/is tracked in this repo and the localnpm run buildchurned it; that churn isdeliberately not included here — only
src/enhance.tsandsrc/__tests__/enhance.test.tsare staged, so the diff stays reviewable.
Merge
Public repo — needs one human CODEOWNER approval; wave-av-agent merges after that once the
exemption lands. Not merging from this branch, and no labels added.
Note
Low Risk
Small, behavior-preserving URL normalization change in SDK config handling; no auth or wire-format changes.
Overview
EnhanceAPI now normalizes
baseUrlwith the sharedstripTrailingSlasheshelper fromurl-utilinstead ofreplace(/\/+$/, ''), aligning with custody, automations, realtime, and runtime and clearing the CodeQL js/polynomial-redos alert on that regex.Behavior is unchanged for typical URLs; the scan is linear instead of backtracking on long trailing-slash runs.
A new
enhance.test.tscase asserts that a client configured withhttps://api.wave.online///still POSTs tohttps://api.wave.online/v1/enhance?model=espcn(without duplicate path slashes in the request URL).Reviewed by Cursor Bugbot for commit c68178e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by Sourcery
Normalize EnhanceAPI base URLs with the shared linear helper and verify trailing-slash handling.
Bug Fixes:
Enhancements:
Tests:
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.