feat: add vibe-utils shared utilities module #73
Conversation
Add foundational utility module for vibe functionality: - HTTP fetch with timeout and retry logic - Exponential backoff for transient failures - URL validation and security checks - Verbose logging utilities - Error handling with error-causes This is the first module in the vibe command implementation, providing shared functionality for authentication, file handling, code generation, and publishing modules. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95b83c835e
ℹ️ 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".
lib/vibe-utils.js
Outdated
| const response = await fetch(url, { | ||
| ...init, | ||
| signal: controller.signal, |
There was a problem hiding this comment.
Because the timeout wrapper always sets signal: controller.signal after spreading init, any init.signal provided by callers is overwritten. That means external cancellation (e.g., user aborting an in‑flight request) is ignored and the request will continue until the timeout fires, which can hang CLI flows that rely on early aborts or misclassify a user‑initiated abort as a timeout. This shows up whenever a caller passes an AbortSignal to fetchWithTimeout or fetchWithRetry expecting it to cancel the request.
Useful? React with 👍 / 👎.
lib/vibe-utils.js
Outdated
| // Reject absolute paths (check both original and normalized) | ||
| if ( | ||
| filePath.startsWith("/") || | ||
| normalizedPath.startsWith("/") || | ||
| /^[A-Za-z]:/.test(filePath) || | ||
| /^[A-Za-z]:/.test(normalizedPath) |
There was a problem hiding this comment.
The absolute‑path check only rejects Unix roots and drive‑letter prefixes, so UNC paths like \\server\share\file are treated as relative and pass validation. On Windows (or any environment that resolves UNC paths as absolute), this allows callers to supply a path that escapes the working directory while still returning { safe: true }, undermining the security intent of the traversal guard.
Useful? React with 👍 / 👎.
|
@cursor review |
|
Skipping Bugbot: Bugbot is disabled for this repository |
Two security fixes for vibe-utils: 1. fetchWithTimeout - AbortSignal combination (P2) - Previously overwrote caller-provided signals with timeout signal - Now uses AbortSignal.any() to combine timeout and external signals - External cancellation (user abort) is now properly respected - Distinguishes timeout aborts from external aborts in error handling 2. isPathSafe - UNC path detection (P2) - Previously only checked for Unix and Windows drive letter absolute paths - UNC paths like \server\share bypassed validation - Now detects UNC paths (\\) as absolute paths and rejects them Both fixes include comprehensive test coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard. |
Extends absolute path detection to catch single-backslash root-relative paths like \Windows\System32 which resolve to the current drive's root on Windows. Previous check only caught: - Unix absolute: /etc/passwd - Drive letter: C:\Windows - UNC paths: \server\share Now also catches: - Root-relative: \Windows\System32 - Any path starting with backslash TDD approach: tests written first (RED), then implementation (GREEN). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@cursoragent please review |
|
@cursor please review |
|
Hi Braden - why'd you close this? I was hoping to review this stuff with you and get it merged. Can you book another meeting with me to discuss? I'm excited to move this partnership forward. |
2 participants
Summary
vibe-utilsmodule with shared utilities for vibe functionalityPR 1 of 5 - Vibe Command Implementation
This is the first in a series of PRs implementing the
--vibeCLI command:Test plan
🤖 Generated with Claude Code