Repository navigation
fix(bin): validate ANTIGRAVITY_LS_ADDRESS port strictly (#494) - #495
Conversation
Bare parseInt turned abc/empty into NaN, let 99999/0/-5 through, and silently truncated 18745abc/18745.9 to 18745; the failed heartbeat was swallowed and the proxy fell through to spawning a managed daemon instead of naming the malformed address. Parse the address through a new strict parseAddressPortValue in the shared proxy-port helper (digits only, integer 1-65535, error names the var and full address), and bound the candidate add() helper to 1-65535 as well. Regression tests cover the malformed spellings and guard against a bare parseInt returning to the address path.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 3
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d58721b92
ℹ️ 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".
| const segment = typeof raw === 'string' ? raw.split(':').at(-1) : undefined; | ||
| if (segment !== undefined && /^[0-9]+$/.test(segment)) { | ||
| const port = Number(segment); | ||
| if (port >= 1 && port <= 65535) return port; |
There was a problem hiding this comment.
Require a complete host:port address
Reject values that merely end in digits rather than treating the final colon-delimited token as sufficient validation. For example, ANTIGRAVITY_LS_ADDRESS=::1, :18745, or even bare 18745 is accepted and interpreted as port 1 or 18745; with a CSRF token set, a failed heartbeat then falls through to starting the managed daemon instead of reporting the malformed explicit address, preserving the behavior this change is intended to prevent.
Useful? React with 👍 / 👎.
Fixes #494
bin/proxy-port.mjs: new strictparseAddressPortValue(name, raw)— port is the segment after the last:, digits only, integer 1-65535; anything else throws an error naming the env var and the full address.bin/antigravity-ls-proxy.mjs: theANTIGRAVITY_LS_ADDRESSbranch ofdetectRunningInstances()now uses that helper instead of a bareparseInt, so a malformed explicit address fails fast instead of being silently discarded (failed heartbeat swallowed → managed daemon spawned). The candidateadd()helper now also bounds ports to1-65535(was only> 0).bin/proxy-port.test.mjs: regression tests for the malformed spellings (abc, empty,0,-5,99999,65536,18745abc,18745.9, leading space) plus a source guard against a bareparseIntreturning to the address path; the bin proxies: *_PROXY_PORT parsed with bare parseInt — invalid values crash opaquely or bind the wrong port #483 import guard now tolerates multi-name imports from the shared helper.Verified locally:
node --test bin/proxy-port.test.mjs— 9/9 pass (was 7 tests; the 2 new ones fail against the pre-fix sources by construction: the helper did not exist and the address path usedparseInt(parts…)).Need help on this PR? Tag
@codesmith-botwith what you need.Summary by CodeRabbit