adding support for UDS proxies #2901
Conversation
Reviewer's GuideThis PR extends proxy support to Unix Domain Sockets by refactoring proxy initialization into a dedicated module, introducing a TCP-UDS bridge with authentication, updating the network settings UI for UDS proxies, and augmenting build/dependency configurations and utility scripts to support the new workflow. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughAdds UDS proxy support across Electron app and settings: new proxy setup module with TCP↔UDS bridge and auth header; settings schema updated; React settings UI overhauled to Flowbite React with new plugins/config; legacy static settings UI/CSS removed; added scripts to experiment with UDS/HTTP bridging; workspace updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SettingsUI as Settings UI (React)
participant Main as Electron Main
participant Proxy as setupProxy
participant Bridge as TcpUdsBridge
participant Sess as Electron session
User->>SettingsUI: Select "UDS proxy" + socket_path
SettingsUI->>Main: IPC UPDATE_SETTINGS_DATA
Main->>Proxy: setupProxy(settings)
Proxy->>Main: await app.whenReady()
alt uds_proxy
Proxy->>Bridge: new TcpUdsBridge(socket_path)
Bridge-->>Proxy: start() → {port, authToken}
Proxy->>Sess: setProxy({proxyRules: "http=127.0.0.1:port", bypass: "localhost"})
Proxy->>Sess: onBeforeSendHeaders inject x-bridge-auth
Proxy->>Main: app.on("before-quit") → Bridge.close()
else other modes
Proxy->>Sess: setProxy({...})
end
Proxy->>Sess: resolveProxy(http://localhost)
sequenceDiagram
autonumber
participant Client as HTTP Client (Electron)
participant Sess as Electron session
participant Bridge as TcpUdsBridge (TCP)
participant UDS as Upstream UDS Proxy
Client->>Sess: Request http://example.com
Sess->>Bridge: TCP connect 127.0.0.1:PORT with headers + x-bridge-auth
Bridge->>Bridge: Validate auth header (constant-time compare)
alt Auth OK
Bridge->>UDS: Connect socket_path
Bridge->>UDS: Forward sanitized request
UDS-->>Bridge: Response stream
Bridge-->>Sess: Response stream
else Auth Fail
Bridge-->>Sess: 401 Unauthorized
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @imolorhe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the application's networking capabilities by integrating support for Unix Domain Socket (UDS) proxies. It involves a comprehensive overhaul of the proxy configuration mechanism, a major upgrade of the UI component library for settings, and the introduction of a robust TCP-UDS bridging solution. These changes provide users with more flexible and secure proxy options, particularly for local inter-process communication. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }; | ||
|
|
||
| // Forward the request | ||
| const proxyReq = request(options, (proxyRes) => { |
Check warning
Code scanning / SonarCloud
Server-side requests should not be vulnerable to forging attacks Medium
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `packages/altair-electron-settings/src/NetworkSettings.tsx:93-95` </location>
<code_context>
/>
+ <Label htmlFor="proxy-unixsocket">Use Unix domain socket proxy</Label>
+ </div>
+ {watch('proxy_setting') === 'uds_proxy' && (
+ <div className="ml-8 nested flex flex-col gap-2">
+ <TextInput
</code_context>
<issue_to_address>
**suggestion:** Consider validating socket_path input for common path errors.
Since users can enter any value, adding validation or warnings for invalid paths (such as those not starting with '/') can help prevent configuration errors.
```suggestion
+ {watch('proxy_setting') === 'uds_proxy' && (
+ <div className="ml-8 nested flex flex-col gap-2">
+ <TextInput
+ label="Socket path"
+ {...register('socket_path', {
+ validate: (value: string) => {
+ if (!value.startsWith('/')) {
+ return 'Socket path should start with "/"';
+ }
+ if (value.trim() === '') {
+ return 'Socket path cannot be empty';
+ }
+ // Add more validation rules as needed
+ return true;
+ },
+ })}
+ error={!!errors.socket_path}
+ helperText={errors.socket_path?.message}
+ />
```
</issue_to_address>
### Comment 2
<location> `packages/altair-electron-settings/src/NetworkSettings.tsx:11-12` </location>
<code_context>
- ipc.sendSync(SETTINGS_STORE_EVENTS.GET_SETTINGS_DATA) || {};
- console.log(initialData);
+ const initialData = ipc.sendSync(SETTINGS_STORE_EVENTS.GET_SETTINGS_DATA) || {};
+ log(initialData);
const { register, handleSubmit, watch } = useForm<SettingStore['settings']>({
defaultValues: initialData,
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider removing or gating log statements in production.
Logging initialData may reveal sensitive information. Please ensure log statements are disabled or gated in production environments.
```suggestion
const initialData = ipc.sendSync(SETTINGS_STORE_EVENTS.GET_SETTINGS_DATA) || {};
if (process.env.NODE_ENV !== 'production') {
log(initialData);
}
```
</issue_to_address>
### Comment 3
<location> `packages/altair-electron-settings/src/NetworkSettings.tsx:17` </location>
<code_context>
- const onSubmit: SubmitHandler<SettingStore['settings']> = data => {
- console.log(data);
+ const onSubmit: SubmitHandler<SettingStore['settings']> = (data) => {
+ log(data);
ipc.sendSync(SETTINGS_STORE_EVENTS.UPDATE_SETTINGS_DATA, data);
ipc.sendSync('from-renderer:restart-app');
</code_context>
<issue_to_address>
**🚨 issue (security):** Logging submitted settings data may leak sensitive information.
Limit logged fields to avoid exposing sensitive data, or remove this log in production.
</issue_to_address>
### Comment 4
<location> `packages/altair-electron/src/proxy/tcp-uds-bridge.ts:108` </location>
<code_context>
+ });
+ });
+
+ process.on('SIGINT', () => {
+ log('Shutting down bridge...');
+ this.server.close(() => process.exit(0));
</code_context>
<issue_to_address>
**suggestion (bug_risk):** SIGINT handler may interfere with Electron's shutdown sequence.
Using process.exit(0) in the SIGINT handler can bypass Electron's cleanup routines. It's better to use Electron's app lifecycle events for shutdown to ensure proper resource management.
Suggested implementation:
```typescript
// Use Electron's app lifecycle events for shutdown to ensure proper resource management
const { app } = require('electron');
app.on('before-quit', () => {
log('Shutting down bridge...');
this.server.close();
});
```
- Make sure that this file has access to Electron's `app` instance. If this code is not running in the main process, you may need to communicate with the main process to trigger the shutdown.
- Remove any other direct calls to `process.exit()` related to shutdown in this file.
- If you want to handle other shutdown signals (like `SIGTERM`), consider adding similar handlers using Electron's lifecycle events.
</issue_to_address>
### Comment 5
<location> `packages/altair-electron/src/proxy/tcp-uds-bridge.ts:120` </location>
<code_context>
+ if (this.server.listening) {
+ this.server.close();
+ }
+ // // Handle server errors - reject
+ // this.server.once('error', (err) => {
+ // if (err.code === 'EADDRINUSE') {
</code_context>
<issue_to_address>
**issue (bug_risk):** Commented-out error handling for server.listen should be implemented.
Uncomment and update the error handling to catch server startup issues like EADDRINUSE, so failures are reported and managed appropriately.
</issue_to_address>
### Comment 6
<location> `packages/altair-electron/src/proxy/tcp-uds-bridge.ts:1` </location>
<code_context>
+import net from 'net';
+import { timingSafeEqual, randomBytes } from 'crypto';
+import getPort from 'get-port';
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring to use http.createServer for automatic HTTP parsing and header management.
You can completely eliminate your manual-buffering + header-splitting logic by switching from `net.createServer` → `http.createServer`. Node will do all the parsing for you, expose a proper `req.headers` object (so you don’t need `processAndAuthenticateRequest` at all), let you write a 401 with one call, and still give you access to the raw socket for piping. Here’s a minimal refactor:
```ts
import http from 'http';
import net from 'net';
import { randomBytes, timingSafeEqual } from 'crypto';
export class TcpUdsBridge {
private server: http.Server;
private authToken = randomBytes(32).toString('hex');
constructor(private udsPath: string) {
this.server = http.createServer(this.onRequest.bind(this));
process.on('SIGINT', () => this.close());
}
async start() {
const port = await getPort();
await new Promise<void>((resolve) =>
this.server.listen(port, '127.0.0.1', resolve)
);
return { port, authToken: this.authToken };
}
close() {
this.server.close();
}
private onRequest(req: http.IncomingMessage, res: http.ServerResponse) {
const auth = req.headers['x-bridge-auth'];
if (
typeof auth !== 'string' ||
auth.length !== this.authToken.length ||
!timingSafeEqual(Buffer.from(auth), Buffer.from(this.authToken))
) {
return res.writeHead(401, { 'Content-Type': 'text/plain' }).end(
'Unauthorized'
);
}
// strip the header so it never reaches the UDS
delete req.headers['x-bridge-auth'];
// build the sanitized start-line + headers
const startLine = `${req.method} ${req.url} HTTP/${req.httpVersion}\r\n`;
const headerLines = Object.entries(req.headers)
.map(([k, v]) => `${k}: ${v}`)
.join('\r\n');
const preamble = startLine + headerLines + '\r\n\r\n';
// connect to UDS and forward everything
const uds = net.connect(this.udsPath);
uds.write(preamble);
req.pipe(uds);
uds.pipe((req.socket as net.Socket));
uds.on('error', (err) => {
res.socket.destroy();
});
}
}
```
What this gives you:
- No manual buffer/`\r\n\r\n` detection
- Clean `req.headers` and `res.writeHead(…)`
- Single‐purpose `onRequest` instead of a big state machine
- Full bidirectional piping on the raw sockets without extra event listeners
- Zero need for a `Map` of connections or a custom `processAndAuthenticateRequest`
All existing functionality is preserved, but each piece is now tiny, testable, and using Node’s built-in HTTP parser.
</issue_to_address>
### Comment 7
<location> `scripts/uds-to-http.ts:22` </location>
<code_context>
+if (existsSync(SOCKET_PATH)) unlinkSync(SOCKET_PATH);
+
+// Create a simple HTTP proxy server
+const server = createServer((clientSocket) => {
+ let buffer = '';
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing manual socket and HTTP parsing with Node's built-in HTTP server to simplify the proxy implementation.
Here’s how you can collapse all of the manual buffering/parsing/formatting into Node’s built-in HTTP server (still over a UDS) with identical behavior:
```js
import { createServer as createHttpServer, request as httpRequest } from 'http';
import { existsSync, unlinkSync } from 'fs';
const SOCKET_PATH = '/tmp/http-proxy.sock';
if (existsSync(SOCKET_PATH)) unlinkSync(SOCKET_PATH);
const server = createHttpServer((req, res) => {
const { method, headers, url, httpVersion } = req;
// enforce Host header
const host = headers.host;
if (!host) {
res.writeHead(400);
return res.end('Missing Host header');
}
// reconstruct full URL if needed
const target = url.startsWith('http') ? url : `http://${host}${url}`;
const proxyReq = httpRequest(target, { method, headers }, proxyRes => {
// pipe status, headers, and body right back
res.writeHead(proxyRes.statusCode, proxyRes.statusMessage, proxyRes.headers);
proxyRes.pipe(res);
});
proxyReq.on('error', err => {
res.writeHead(502);
res.end(err.message);
});
// forward client body
req.pipe(proxyReq);
});
server.listen(SOCKET_PATH, () => {
console.log(`🟢 UDS HTTP proxy listening at ${SOCKET_PATH}`);
});
```
What this changes:
- Drops the manual `net.createServer` + buffer/split logic.
- Uses `req.url`, `req.headers`, `req.method`, `req.httpVersion`.
- Automatically handles chunking/streaming for you via `req.pipe()`/`proxyRes.pipe()`.
- Preserves identical functionality (400 on missing Host, 502 on request‐errors, full header passage, UDS listen).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| {watch('proxy_setting') === 'uds_proxy' && ( | ||
| <div className="ml-8 nested flex flex-col gap-2"> | ||
| <TextInput |
There was a problem hiding this comment.
suggestion: Consider validating socket_path input for common path errors.
Since users can enter any value, adding validation or warnings for invalid paths (such as those not starting with '/') can help prevent configuration errors.
| {watch('proxy_setting') === 'uds_proxy' && ( | |
| <div className="ml-8 nested flex flex-col gap-2"> | |
| <TextInput | |
| + {watch('proxy_setting') === 'uds_proxy' && ( | |
| + <div className="ml-8 nested flex flex-col gap-2"> | |
| + <TextInput | |
| + label="Socket path" | |
| + {...register('socket_path', { | |
| + validate: (value: string) => { | |
| + if (!value.startsWith('/')) { | |
| + return 'Socket path should start with "/"'; | |
| + } | |
| + if (value.trim() === '') { | |
| + return 'Socket path cannot be empty'; | |
| + } | |
| + // Add more validation rules as needed | |
| + return true; | |
| + }, | |
| + })} | |
| + error={!!errors.socket_path} | |
| + helperText={errors.socket_path?.message} | |
| + /> |
| const onSubmit: SubmitHandler<SettingStore['settings']> = data => { | ||
| console.log(data); | ||
| const onSubmit: SubmitHandler<SettingStore['settings']> = (data) => { | ||
| log(data); |
There was a problem hiding this comment.
🚨 issue (security): Logging submitted settings data may leak sensitive information.
Limit logged fields to avoid exposing sensitive data, or remove this log in production.
| }); | ||
| }); | ||
|
|
||
| process.on('SIGINT', () => { |
There was a problem hiding this comment.
suggestion (bug_risk): SIGINT handler may interfere with Electron's shutdown sequence.
Using process.exit(0) in the SIGINT handler can bypass Electron's cleanup routines. It's better to use Electron's app lifecycle events for shutdown to ensure proper resource management.
Suggested implementation:
// Use Electron's app lifecycle events for shutdown to ensure proper resource management
const { app } = require('electron');
app.on('before-quit', () => {
log('Shutting down bridge...');
this.server.close();
});- Make sure that this file has access to Electron's
appinstance. If this code is not running in the main process, you may need to communicate with the main process to trigger the shutdown. - Remove any other direct calls to
process.exit()related to shutdown in this file. - If you want to handle other shutdown signals (like
SIGTERM), consider adding similar handlers using Electron's lifecycle events.
| if (this.server.listening) { | ||
| this.server.close(); | ||
| } | ||
| // // Handle server errors - reject |
There was a problem hiding this comment.
issue (bug_risk): Commented-out error handling for server.listen should be implemented.
Uncomment and update the error handling to catch server startup issues like EADDRINUSE, so failures are reported and managed appropriately.
| @@ -0,0 +1,217 @@ | |||
| import net from 'net'; | |||
There was a problem hiding this comment.
issue (complexity): Consider refactoring to use http.createServer for automatic HTTP parsing and header management.
You can completely eliminate your manual-buffering + header-splitting logic by switching from net.createServer → http.createServer. Node will do all the parsing for you, expose a proper req.headers object (so you don’t need processAndAuthenticateRequest at all), let you write a 401 with one call, and still give you access to the raw socket for piping. Here’s a minimal refactor:
import http from 'http';
import net from 'net';
import { randomBytes, timingSafeEqual } from 'crypto';
export class TcpUdsBridge {
private server: http.Server;
private authToken = randomBytes(32).toString('hex');
constructor(private udsPath: string) {
this.server = http.createServer(this.onRequest.bind(this));
process.on('SIGINT', () => this.close());
}
async start() {
const port = await getPort();
await new Promise<void>((resolve) =>
this.server.listen(port, '127.0.0.1', resolve)
);
return { port, authToken: this.authToken };
}
close() {
this.server.close();
}
private onRequest(req: http.IncomingMessage, res: http.ServerResponse) {
const auth = req.headers['x-bridge-auth'];
if (
typeof auth !== 'string' ||
auth.length !== this.authToken.length ||
!timingSafeEqual(Buffer.from(auth), Buffer.from(this.authToken))
) {
return res.writeHead(401, { 'Content-Type': 'text/plain' }).end(
'Unauthorized'
);
}
// strip the header so it never reaches the UDS
delete req.headers['x-bridge-auth'];
// build the sanitized start-line + headers
const startLine = `${req.method} ${req.url} HTTP/${req.httpVersion}\r\n`;
const headerLines = Object.entries(req.headers)
.map(([k, v]) => `${k}: ${v}`)
.join('\r\n');
const preamble = startLine + headerLines + '\r\n\r\n';
// connect to UDS and forward everything
const uds = net.connect(this.udsPath);
uds.write(preamble);
req.pipe(uds);
uds.pipe((req.socket as net.Socket));
uds.on('error', (err) => {
res.socket.destroy();
});
}
}What this gives you:
- No manual buffer/
\r\n\r\ndetection - Clean
req.headersandres.writeHead(…) - Single‐purpose
onRequestinstead of a big state machine - Full bidirectional piping on the raw sockets without extra event listeners
- Zero need for a
Mapof connections or a customprocessAndAuthenticateRequest
All existing functionality is preserved, but each piece is now tiny, testable, and using Node’s built-in HTTP parser.
| if (existsSync(SOCKET_PATH)) unlinkSync(SOCKET_PATH); | ||
|
|
||
| // Create a simple HTTP proxy server | ||
| const server = createServer((clientSocket) => { |
There was a problem hiding this comment.
issue (complexity): Consider replacing manual socket and HTTP parsing with Node's built-in HTTP server to simplify the proxy implementation.
Here’s how you can collapse all of the manual buffering/parsing/formatting into Node’s built-in HTTP server (still over a UDS) with identical behavior:
import { createServer as createHttpServer, request as httpRequest } from 'http';
import { existsSync, unlinkSync } from 'fs';
const SOCKET_PATH = '/tmp/http-proxy.sock';
if (existsSync(SOCKET_PATH)) unlinkSync(SOCKET_PATH);
const server = createHttpServer((req, res) => {
const { method, headers, url, httpVersion } = req;
// enforce Host header
const host = headers.host;
if (!host) {
res.writeHead(400);
return res.end('Missing Host header');
}
// reconstruct full URL if needed
const target = url.startsWith('http') ? url : `http://${host}${url}`;
const proxyReq = httpRequest(target, { method, headers }, proxyRes => {
// pipe status, headers, and body right back
res.writeHead(proxyRes.statusCode, proxyRes.statusMessage, proxyRes.headers);
proxyRes.pipe(res);
});
proxyReq.on('error', err => {
res.writeHead(502);
res.end(err.message);
});
// forward client body
req.pipe(proxyReq);
});
server.listen(SOCKET_PATH, () => {
console.log(`🟢 UDS HTTP proxy listening at ${SOCKET_PATH}`);
});What this changes:
- Drops the manual
net.createServer+ buffer/split logic. - Uses
req.url,req.headers,req.method,req.httpVersion. - Automatically handles chunking/streaming for you via
req.pipe()/proxyRes.pipe(). - Preserves identical functionality (400 on missing Host, 502 on request‐errors, full header passage, UDS listen).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully adds support for UDS proxies by introducing a TCP-to-UDS bridge and updating the settings UI. The code is well-structured, particularly the refactoring of proxy logic into its own module. My review focuses on improving robustness and error handling. I've identified a critical issue with unhandled server errors that could crash the application, a high-severity issue where misconfiguration can prevent the app from starting, and a medium-severity issue regarding a redundant process signal handler. Addressing these points will enhance the stability and user experience of the new feature.
| // // Handle server errors - reject | ||
| // this.server.once('error', (err) => { | ||
| // if (err.code === 'EADDRINUSE') { | ||
| // error(`Port ${port} is already in use`); | ||
| // } else { | ||
| // error(`Server error: ${err.message}`); | ||
| // } | ||
| // process.exit(1); | ||
| // }); |
There was a problem hiding this comment.
The error handling for the server in the start method is commented out. If getPort() returns a port that becomes occupied before this.server.listen() is called, it will throw an EADDRINUSE error. Without this handler, the error will be unhandled and will crash the application's main process. This is a critical stability issue. Please uncomment this block to handle potential port conflicts gracefully. While process.exit(1) is abrupt for a GUI application, it's better than an unhandled crash.
| // // Handle server errors - reject | |
| // this.server.once('error', (err) => { | |
| // if (err.code === 'EADDRINUSE') { | |
| // error(`Port ${port} is already in use`); | |
| // } else { | |
| // error(`Server error: ${err.message}`); | |
| // } | |
| // process.exit(1); | |
| // }); | |
| // Handle server errors - reject | |
| this.server.once('error', (err) => { | |
| if ((err as NodeJS.ErrnoException).code === 'EADDRINUSE') { | |
| error(`Port ${port} is already in use`); | |
| } else { | |
| error(`Server error: ${err.message}`); | |
| } | |
| process.exit(1); | |
| }); |
| if (!settings.socket_path) { | ||
| throw new Error('Socket path is required for UDS proxy'); | ||
| } |
There was a problem hiding this comment.
Throwing an error here when the socket path is missing will crash the main process on startup if the user has selected the UDS proxy option but not provided a path. This results in a poor user experience, as the app will fail to launch without a clear explanation to the user. It would be better to handle this gracefully by showing an error dialog to the user and falling back to a 'no proxy' configuration. You will need to import dialog from electron at the top of the file.
| if (!settings.socket_path) { | |
| throw new Error('Socket path is required for UDS proxy'); | |
| } | |
| if (!settings.socket_path) { | |
| dialog.showErrorBox( | |
| 'Proxy Configuration Error', | |
| 'A socket path is required for UDS proxy but is missing. Altair will use no proxy instead.' | |
| ); | |
| proxyConfig.mode = 'direct'; | |
| break; | |
| } |
| process.on('SIGINT', () => { | ||
| log('Shutting down bridge...'); | ||
| this.server.close(() => process.exit(0)); | ||
| }); |
There was a problem hiding this comment.
This process.on('SIGINT', ...) handler seems intended for a standalone Node.js script, but this class is used within an Electron application. The Electron app lifecycle is managed by app events, and cleanup is already being handled via app.on('before-quit', ...) in proxy/index.ts. This handler is redundant and could cause unexpected behavior, especially with its call to process.exit(0). It's best to remove it to rely solely on the Electron app's lifecycle events for cleanup.
|
Visit the preview URL for this PR (updated for commit 80deac0): https://altair-gql--pr2901-imolorhe-uds-proxy-s-4bb8shpb.web.app (expires Sun, 12 Oct 2025 23:57:19 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 02d6323d75a99e532a38922862e269d63351a6cf |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
scripts/test-uds-proxy.ts (1)
5-14: Add error handling for the HTTP request.The script lacks error handling, which can make debugging difficult if the socket connection fails or the request encounters issues.
Apply this diff to add proper error handling:
http.get( { socketPath: '/tmp/http-proxy.sock', path: 'http://example.com/', headers: { Host: 'example.com' }, }, (res) => { res.pipe(process.stdout); + res.on('error', (err) => { + console.error('Response error:', err); + process.exit(1); + }); } -); +).on('error', (err) => { + console.error('Request error:', err); + process.exit(1); +});scripts/package.json (3)
5-5: Remove or update the main field.The
mainfield points toindex.js, which doesn't exist in this package. Since this is a scripts package without a public entry point, consider removing this field or updating it to match the actual structure.Apply this diff to remove the unnecessary main field:
"version": "1.0.0", "description": "", - "main": "index.js", "scripts": {
1-17: Consider marking the package as private.Since this is an internal scripts package not meant for publication, it should be marked as private to prevent accidental publishing.
Apply this diff to add the private field:
{ "name": "scripts", "version": "1.0.0", "description": "", "main": "index.js", + "private": true, "scripts": {
12-12: scripts/package.json’s @types/node version is ^24.6.2, newer than the root engine (>=20) and most workspace packages. Confirm your runtime supports Node v24 APIs or consolidate @types/node to a version aligned across the monorepo.scripts/tsconfig.json (1)
1-44: LGTM! TypeScript configuration is appropriate for Node.js scripts.The configuration properly extends the base tsconfig and sets up Node.js-specific options with strict type checking enabled. The
nodenextmodule resolution andesnexttarget are appropriate for modern Node.js development.Optional: Consider enabling additional strict options.
Lines 29-34 have several commented style options that could improve code quality:
- // "noImplicitReturns": true, - // "noImplicitOverride": true, - // "noUnusedLocals": true, - // "noUnusedParameters": true, - // "noFallthroughCasesInSwitch": true, - // "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noFallthroughCasesInSwitch": true,scripts/uds-to-http.ts (1)
16-16: Consider making socket path configurable.The socket path is hardcoded, which limits flexibility for testing different configurations.
Make the socket path configurable via environment variable:
-const SOCKET_PATH = '/tmp/http-proxy.sock'; +const SOCKET_PATH = process.env.SOCKET_PATH || '/tmp/http-proxy.sock';Then update the usage comments to mention the environment variable:
* For testing purposes. For a simple test, you can use curl: + * You can customize the socket path with SOCKET_PATH environment variable. * * curl --unix-socket /tmp/http-proxy.sock http://example.com/scripts/uds-server.ts (1)
1-38: LGTM! Simple and clear test server implementation.This is a straightforward UDS test server that serves its purpose well. The code is clear, properly handles socket cleanup, and includes helpful usage examples.
Optional: Make socket path configurable for consistency.
To match the pattern that could be applied in
uds-to-http.ts, consider making the socket path configurable:-const SOCKET_PATH = '/tmp/test-destination.sock'; +const SOCKET_PATH = process.env.SOCKET_PATH || '/tmp/test-destination.sock';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/altair-electron/static/settings/fonts/photon-entypo.eotis excluded by!**/*.eotpackages/altair-electron/static/settings/fonts/photon-entypo.svgis excluded by!**/*.svgpackages/altair-electron/static/settings/fonts/photon-entypo.ttfis excluded by!**/*.ttfpackages/altair-electron/static/settings/fonts/photon-entypo.woffis excluded by!**/*.woffpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
packages/altair-electron-interop/src/settings.ts(1 hunks)packages/altair-electron-settings/.flowbite-react/.gitignore(1 hunks)packages/altair-electron-settings/.flowbite-react/config.json(1 hunks)packages/altair-electron-settings/.flowbite-react/init.tsx(1 hunks)packages/altair-electron-settings/package.json(1 hunks)packages/altair-electron-settings/src/App.tsx(2 hunks)packages/altair-electron-settings/src/NetworkSettings.tsx(1 hunks)packages/altair-electron-settings/src/log.ts(1 hunks)packages/altair-electron-settings/tailwind.config.js(1 hunks)packages/altair-electron-settings/vite.config.ts(1 hunks)packages/altair-electron/src/app/index.ts(3 hunks)packages/altair-electron/src/proxy/index.ts(1 hunks)packages/altair-electron/src/proxy/tcp-uds-bridge.ts(1 hunks)packages/altair-electron/static/settings/app.js(0 hunks)packages/altair-electron/static/settings/css/photon.css(0 hunks)packages/altair-electron/static/settings/css/photon.min.css(0 hunks)packages/altair-electron/static/settings/css/settings.css(0 hunks)packages/altair-electron/static/settings/index.html(0 hunks)pnpm-workspace.yaml(1 hunks)scripts/http-to-uds.ts(1 hunks)scripts/package.json(1 hunks)scripts/test-uds-proxy.ts(1 hunks)scripts/tsconfig.json(1 hunks)scripts/typescript(1 hunks)scripts/uds-server.ts(1 hunks)scripts/uds-to-http.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/altair-electron/static/settings/index.html
- packages/altair-electron/static/settings/css/photon.min.css
- packages/altair-electron/static/settings/css/settings.css
- packages/altair-electron/static/settings/app.js
- packages/altair-electron/static/settings/css/photon.css
🧰 Additional context used
📓 Path-based instructions (6)
**/tsconfig*.json
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/tsconfig*.json: Use TypeScript's strict mode for better type safety
Keep TypeScript configuration consistent across packages
Use strict TypeScript settings for better type safety
Files:
scripts/tsconfig.json
**/*.ts
📄 CodeRabbit inference engine (.github/instructions/app-testing.instructions.md)
Follow project code style using ESLint and Prettier
**/*.ts: Use explicit type annotations for function parameters and return types
Prefer interfaces over type aliases for object shapes
Use union types and literal types for better type safety
Leverage generic types for reusable components
Group imports: external libraries first, then internal modules
Use absolute imports from package roots when possible
Prefer named exports over default exports
Use custom error classes that extend Error
Implement proper error boundaries and handling
Log errors with sufficient context for debugging
Use observables (RxJS) for reactive programming patterns where appropriate
Manage subscriptions to avoid memory leaks
Use appropriate RxJS operators for data transformation
Handle errors in observable streams
Use async/await for sequential operations
Handle promise rejections properly
Use Promise.all() for concurrent operations
Implement timeout handling for long-running operations
Dispose of resources properly (subscriptions, event listeners)
Use weak references where appropriate
Avoid creating unnecessary objects in hot paths
Profile memory usage for performance-critical code
Use tree-shaking-friendly imports
Lazy load heavy modules when possible
Monitor bundle size impacts of new dependencies
Use dynamic imports for code splitting
Validate and sanitize all user inputs
Implement proper XSS and injection prevention
Validate API responses before processing
Sanitize sensitive data in logs
Follow secure coding practices
Group related functionality in modules
Keep files focused and not too large
Use consistent naming conventions
Organize imports and exports clearly
Write JSDoc comments for public APIs
Keep documentation up to date with code changes (inline docs)
Use meaningful variable and function names
Handle environment-specific APIs properly
Use TypeScript features appropriate for the configured version
Files:
scripts/test-uds-proxy.tspackages/altair-electron-settings/src/log.tspackages/altair-electron/src/proxy/index.tsscripts/uds-server.tspackages/altair-electron/src/app/index.tspackages/altair-electron-interop/src/settings.tspackages/altair-electron/src/proxy/tcp-uds-bridge.tsscripts/uds-to-http.tspackages/altair-electron-settings/vite.config.tsscripts/http-to-uds.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Use TypeScript for implementation across the codebase
Files:
scripts/test-uds-proxy.tspackages/altair-electron-settings/src/log.tspackages/altair-electron-settings/src/NetworkSettings.tsxpackages/altair-electron/src/proxy/index.tspackages/altair-electron-settings/src/App.tsxscripts/uds-server.tspackages/altair-electron/src/app/index.tspackages/altair-electron-interop/src/settings.tspackages/altair-electron/src/proxy/tcp-uds-bridge.tsscripts/uds-to-http.tspackages/altair-electron-settings/vite.config.tsscripts/http-to-uds.ts
packages/altair-electron/**
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Build desktop features using Electron (main/renderer, window management) in packages/altair-electron
Files:
packages/altair-electron/src/proxy/index.tspackages/altair-electron/src/app/index.tspackages/altair-electron/src/proxy/tcp-uds-bridge.ts
**/*config.{js,mjs,cjs}
📄 CodeRabbit inference engine (.github/instructions/javascript.instructions.md)
**/*config.{js,mjs,cjs}: Use consistent formatting and organization in JavaScript configuration files
Include comments explaining complex configurations in JS config files
Validate configuration values where possible in JS config files
Use environment variables for environment-specific settings in config files
Files:
packages/altair-electron-settings/tailwind.config.js
**/*.{js,mjs,cjs}
📄 CodeRabbit inference engine (.github/instructions/javascript.instructions.md)
**/*.{js,mjs,cjs}: Always handle promise rejections
Use try/catch blocks appropriately around async/await
Provide meaningful error messages
Log errors with sufficient context
Convert legacy code to ES6+ features when possible
Use const/let instead of var
Implement proper error handling in legacy code
Add JSDoc comments for better documentation
Explain complex logic and business rules in code comments
Document function parameters and return values
Include usage examples for utilities
Keep comments up to date with code changes
Files:
packages/altair-electron-settings/tailwind.config.js
🧠 Learnings (10)
📚 Learning: 2025-10-03T16:42:34.287Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-10-03T16:42:34.287Z
Learning: Applies to **/tsconfig*.json : Keep TypeScript configuration consistent across packages
Applied to files:
scripts/tsconfig.json
📚 Learning: 2025-10-03T16:39:27.517Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/main.instructions.md:0-0
Timestamp: 2025-10-03T16:39:27.517Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript for implementation across the codebase
Applied to files:
scripts/tsconfig.json
📚 Learning: 2025-10-03T16:42:34.287Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-10-03T16:42:34.287Z
Learning: Applies to **/tsconfig*.json : Use strict TypeScript settings for better type safety
Applied to files:
scripts/tsconfig.json
📚 Learning: 2025-10-03T16:42:34.287Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-10-03T16:42:34.287Z
Learning: Applies to **/tsconfig*.json : Use TypeScript's strict mode for better type safety
Applied to files:
scripts/tsconfig.json
📚 Learning: 2025-10-03T16:42:34.287Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-10-03T16:42:34.287Z
Learning: Applies to **/*.ts : Use TypeScript features appropriate for the configured version
Applied to files:
scripts/tsconfig.json
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.{service,controller}.ts : Use structured logging with appropriate levels
Applied to files:
packages/altair-electron-settings/src/log.ts
📚 Learning: 2025-10-03T16:39:27.517Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/main.instructions.md:0-0
Timestamp: 2025-10-03T16:39:27.517Z
Learning: Use pnpm for monorepo package management and scripts
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-10-03T16:39:27.517Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/main.instructions.md:0-0
Timestamp: 2025-10-03T16:39:27.517Z
Learning: Applies to packages/altair-electron/** : Build desktop features using Electron (main/renderer, window management) in packages/altair-electron
Applied to files:
packages/altair-electron/src/app/index.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/index.ts : Use barrel exports (index.ts) for clean imports
Applied to files:
packages/altair-electron/src/app/index.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/main.ts : Use HTTPS in production
Applied to files:
packages/altair-electron/src/app/index.ts
🧬 Code graph analysis (4)
packages/altair-electron-settings/src/NetworkSettings.tsx (2)
packages/altair-electron-interop/src/settings.ts (2)
SETTINGS_STORE_EVENTS(20-25)SettingStore(1-16)packages/altair-electron-settings/src/log.ts (1)
log(1-4)
packages/altair-electron/src/proxy/index.ts (2)
packages/altair-electron-interop/src/settings.ts (1)
SettingStore(1-16)packages/altair-electron/src/proxy/tcp-uds-bridge.ts (2)
TcpUdsBridge(9-217)DEFAULT_BRIDGE_AUTH_HEADER(7-7)
packages/altair-electron/src/app/index.ts (1)
packages/altair-electron/src/proxy/index.ts (1)
setupProxy(6-68)
packages/altair-electron-settings/vite.config.ts (1)
packages/altair-electron-settings/tailwind.config.js (1)
flowbiteReact(1-1)
🪛 GitHub Check: SonarCloud
scripts/uds-to-http.ts
[warning] 62-62: Server-side requests should not be vulnerable to forging attacks
Change this code to not construct the URL from user-controlled data.See more on SonarQube Cloud
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: tests / test (macos-latest)
- GitHub Check: electron / electron (windows-latest)
- GitHub Check: tauri / tauri (ubuntu-22.04)
- GitHub Check: tests / e2e-test (macos-latest)
- GitHub Check: tests / test (ubuntu-latest)
- GitHub Check: electron / electron (macos-latest)
- GitHub Check: tauri / tauri (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: electron / electron (ubuntu-latest)
- GitHub Check: tauri / tauri (macos-latest, --target x86_64-apple-darwin)
- GitHub Check: tests / e2e-test (ubuntu-latest)
- GitHub Check: tauri / tauri (windows-latest)
- GitHub Check: docker / docker
- GitHub Check: sites / Deploy sites
- GitHub Check: tests / api-e2e-test
- GitHub Check: Sourcery review
- GitHub Check: build_and_preview
🔇 Additional comments (9)
pnpm-workspace.yaml (1)
6-6: LGTM!The addition of the scripts package to the workspace is correct and consistent with the new scripts directory introduced in this PR.
packages/altair-electron-settings/.flowbite-react/.gitignore (1)
1-2: LGTM!The gitignore entries for Flowbite React-generated artifacts (
class-list.json) and process ID files (pid) are appropriate and follow standard practices.packages/altair-electron-settings/.flowbite-react/config.json (1)
1-10: LGTM!The Flowbite React configuration is correctly structured, with version 3 aligning with the flowbite ^3.1.2 dependency in package.json.
packages/altair-electron-settings/src/App.tsx (2)
1-6: LGTM!The updated imports correctly reflect the new Flowbite React 0.12.9 API structure, replacing the nested Sidebar.Items pattern with standalone components.
17-21: LGTM!The Sidebar composition correctly uses the new API structure with SidebarItems, SidebarItemGroup, and SidebarItem as standalone components, consistent with Flowbite React 0.12.9.
packages/altair-electron-settings/vite.config.ts (1)
4-4: LGTM! Flowbite React Vite plugin properly integrated.The Flowbite React Vite plugin is correctly imported and added to the plugins array, aligning with the broader Flowbite adoption across the settings UI.
Also applies to: 9-9
packages/altair-electron-settings/.flowbite-react/init.tsx (1)
1-22: Auto-generated file - no manual changes needed.This file is auto-generated by the Flowbite React CLI as indicated by the comments (lines 5-7). The configuration and ThemeInit component are standard Flowbite React boilerplate.
packages/altair-electron-settings/tailwind.config.js (1)
1-1: LGTM! Flowbite React properly integrated with Tailwind CSS.The Flowbite React Tailwind plugin is correctly imported, the generated class list is included in the content array for scanning, and both the Flowbite base plugin and Flowbite React plugin are properly added to the plugins array.
Also applies to: 10-10, 15-15
packages/altair-electron-interop/src/settings.ts (1)
3-9: LGTM – UDS proxy support and validation are correctly implementedThe union now includes
'uds_proxy'andsocket_pathis checked at runtime in packages/altair-electron/src/proxy/index.ts (throws if missing). No further changes needed.
| "flowbite": "^3.1.2", | ||
| "flowbite-react": "^0.12.9", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Flowbite React component API compatibility.
The update from flowbite-react ^0.7.2 to ^0.12.9 is a significant version jump. While the learnings indicate no major breaking changes in 0.12.x, ensure all Flowbite React component usage across the codebase is compatible with the new version.
Run the following script to verify all Flowbite React component imports and usage:
Based on learnings
🏁 Script executed:
#!/bin/bash
# Description: Find all Flowbite React component imports and usage patterns
# to verify compatibility with version 0.12.9
# Find all imports from flowbite-react
echo "=== Flowbite React imports ==="
rg -nP --type=tsx --type=ts "from ['\"]flowbite-react['\"]" -A 2
# Find component usage patterns
echo -e "\n=== Component usage patterns ==="
rg -nP --type=tsx --type=ts "<(Sidebar|Modal|Button|Dropdown|Navbar|Tooltip)" -C 2Length of output: 120
🏁 Script executed:
#!/bin/bash
# Description: Find all Flowbite React component imports and usage patterns
# using glob patterns for .ts and .tsx files
echo "=== Flowbite React imports ==="
rg -nP --glob '*.ts?(x)' "from ['\"]flowbite-react['\"]" -n -C2
echo -e "\n=== Component usage patterns ==="
rg -nP --glob '*.ts?(x)' "<(Sidebar|Modal|Button|Dropdown|Navbar|Tooltip)" -n -C2Length of output: 362
#!/bin/bash
echo "=== Import or require flowbite-react ==="
rg -nP "(?:from|require)\s*\(?['\"]flowbite-react['\"]\)?"
echo -e "\n=== Flowbite component tags usage ==="
rg -nP "<(Sidebar|Modal|Button|Dropdown|Navbar|Tooltip)" -n -C2🏁 Script executed:
#!/bin/bash
echo "=== Import or require flowbite-react ==="
rg -nP "(?:from|require)\s*['\"]flowbite-react['\"]"
echo -e "\n=== Flowbite component usage ==="
rg -nP "<(Sidebar|Modal|Button|Dropdown|Navbar|Tooltip)" -n -C2Length of output: 4105
Update Flowbite-React component API usage
- In plugins/apollo-tracing/src/components/Tracing/TracingRow.tsx replace the legacy
<Tooltip id="…"/>pattern with the v0.12.x API (e.g.<Tooltip content="…" placement="…">…</Tooltip>). - Verify Sidebar in packages/altair-electron-settings/src/App.tsx and Radio, Label, Button, TextInput, Card imports and props in packages/altair-electron-settings/src/NetworkSettings.tsx align with the v0.12.x API.
🤖 Prompt for AI Agents
In packages/altair-electron-settings/package.json around lines 16-17 and
affecting plugins/apollo-tracing/src/components/Tracing/TracingRow.tsx,
packages/altair-electron-settings/src/App.tsx and
packages/altair-electron-settings/src/NetworkSettings.tsx: update Flowbite-React
v0.12.x API usage — replace legacy self-closing <Tooltip id="…"/> instances with
the new wrapper form <Tooltip content="…" placement="…">...</Tooltip> in
TracingRow.tsx (move the tooltip text into content and wrap the target element),
and in App.tsx and NetworkSettings.tsx align imports and props to v0.12.x by
importing the correct components from 'flowbite-react' and adjusting prop
names/structure (e.g., use content/placement for Tooltip, ensure
Radio/Label/Button/TextInput/Card props follow v0.12.x signatures such as
controlled value/onChange handlers and updated variant/size prop names); consult
the v0.12.x docs and update each component usage accordingly so the code
compiles with the new Flowbite-React version.
| <Label htmlFor="proxy-unixsocket">Use Unix domain socket proxy</Label> | ||
| </div> | ||
| {watch('proxy_setting') === 'uds_proxy' && ( | ||
| <div className="ml-8 nested flex flex-col gap-2"> | ||
| <TextInput | ||
| type="text" | ||
| placeholder="Socket path (e.g., /var/run/proxy.sock)" | ||
| {...register('socket_path')} | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Validate the Unix socket path before restarting
Saving uds_proxy without a socket path sends an empty value to the main process, making setupProxy() throw on the next launch and the app never opens. Block submission until a non-empty path is provided so users don't soft-brick their install.
- const { register, handleSubmit, watch } = useForm<SettingStore['settings']>({
- defaultValues: initialData,
- });
+ const {
+ register,
+ handleSubmit,
+ watch,
+ formState: { errors },
+ } = useForm<SettingStore['settings']>({
+ defaultValues: initialData,
+ });
+ const proxySetting = watch('proxy_setting');
@@
- {watch('proxy_setting') === 'uds_proxy' && (
+ {proxySetting === 'uds_proxy' && (
<div className="ml-8 nested flex flex-col gap-2">
<TextInput
type="text"
placeholder="Socket path (e.g., /var/run/proxy.sock)"
- {...register('socket_path')}
+ {...register('socket_path', {
+ validate: (value) =>
+ proxySetting !== 'uds_proxy' || value?.trim()
+ ? true
+ : 'Socket path is required when using a Unix domain socket proxy',
+ })}
/>
+ {errors.socket_path && (
+ <p className="text-sm text-red-600">
+ {errors.socket_path.message}
+ </p>
+ )}
</div>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Label htmlFor="proxy-unixsocket">Use Unix domain socket proxy</Label> | |
| </div> | |
| {watch('proxy_setting') === 'uds_proxy' && ( | |
| <div className="ml-8 nested flex flex-col gap-2"> | |
| <TextInput | |
| type="text" | |
| placeholder="Socket path (e.g., /var/run/proxy.sock)" | |
| {...register('socket_path')} | |
| /> | |
| </div> | |
| )} | |
| <Label htmlFor="proxy-unixsocket">Use Unix domain socket proxy</Label> | |
| </div> | |
| {proxySetting === 'uds_proxy' && ( | |
| <div className="ml-8 nested flex flex-col gap-2"> | |
| <TextInput | |
| type="text" | |
| placeholder="Socket path (e.g., /var/run/proxy.sock)" | |
| {...register('socket_path', { | |
| validate: (value) => | |
| proxySetting !== 'uds_proxy' || value?.trim() | |
| ? true | |
| : 'Socket path is required when using a Unix domain socket proxy', | |
| })} | |
| /> | |
| {errors.socket_path && ( | |
| <p className="text-sm text-red-600"> | |
| {errors.socket_path.message} | |
| </p> | |
| )} | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In packages/altair-electron-settings/src/NetworkSettings.tsx around lines
91-101, the form allows selecting 'uds_proxy' without requiring a socket path
which causes setupProxy() to throw on launch; add validation to block submission
when proxy_setting === 'uds_proxy' and socket_path is empty: mark the TextInput
as required when visible, add a validation rule via register('socket_path', {
required: 'Socket path is required' }) (or call setError on submit if empty),
display the error message inline, and ensure the form submit handler prevents
saving until a non-empty path is provided.
| const settings = store.get('settings'); | ||
| log(settings); | ||
| if (settings) { | ||
| /** | ||
| * @type Electron.Config | ||
| */ | ||
| const proxyConfig: Electron.ProxyConfig = { | ||
| mode: 'direct', | ||
| }; | ||
|
|
||
| switch (settings.proxy_setting) { | ||
| case 'none': | ||
| proxyConfig.mode = 'direct'; | ||
| break; | ||
| case 'autodetect': | ||
| proxyConfig.mode = 'auto_detect'; | ||
| break; | ||
| case 'system': | ||
| proxyConfig.mode = 'system'; | ||
| break; | ||
| case 'pac': | ||
| proxyConfig.mode = 'pac_script'; | ||
| proxyConfig.pacScript = settings.pac_address; | ||
| break; | ||
| case 'proxy_server': | ||
| proxyConfig.mode = 'fixed_servers'; | ||
| proxyConfig.proxyRules = `${settings.proxy_host}:${settings.proxy_port}`; | ||
| break; | ||
| default: | ||
| } | ||
| await session.defaultSession.setProxy(proxyConfig); | ||
| const proxy = await session.defaultSession.resolveProxy('http://localhost'); | ||
| log(proxy, proxyConfig); | ||
| } | ||
| await setupProxy(settings); | ||
|
|
There was a problem hiding this comment.
Handle proxy setup failures to keep the app booting
If setupProxy() throws (e.g., UDS selected with a missing socket path), the rejection stops window creation and leaves the app without a UI. Surface the error and exit gracefully instead of letting the promise reject unhandled.
- await setupProxy(settings);
+ try {
+ await setupProxy(settings);
+ } catch (proxyError) {
+ error('Error setting up proxy', proxyError);
+ dialog.showErrorBox(
+ 'Proxy configuration error',
+ proxyError instanceof Error
+ ? proxyError.message
+ : String(proxyError)
+ );
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const settings = store.get('settings'); | |
| log(settings); | |
| if (settings) { | |
| /** | |
| * @type Electron.Config | |
| */ | |
| const proxyConfig: Electron.ProxyConfig = { | |
| mode: 'direct', | |
| }; | |
| switch (settings.proxy_setting) { | |
| case 'none': | |
| proxyConfig.mode = 'direct'; | |
| break; | |
| case 'autodetect': | |
| proxyConfig.mode = 'auto_detect'; | |
| break; | |
| case 'system': | |
| proxyConfig.mode = 'system'; | |
| break; | |
| case 'pac': | |
| proxyConfig.mode = 'pac_script'; | |
| proxyConfig.pacScript = settings.pac_address; | |
| break; | |
| case 'proxy_server': | |
| proxyConfig.mode = 'fixed_servers'; | |
| proxyConfig.proxyRules = `${settings.proxy_host}:${settings.proxy_port}`; | |
| break; | |
| default: | |
| } | |
| await session.defaultSession.setProxy(proxyConfig); | |
| const proxy = await session.defaultSession.resolveProxy('http://localhost'); | |
| log(proxy, proxyConfig); | |
| } | |
| await setupProxy(settings); | |
| const settings = store.get('settings'); | |
| try { | |
| await setupProxy(settings); | |
| } catch (proxyError) { | |
| error('Error setting up proxy', proxyError); | |
| dialog.showErrorBox( | |
| 'Proxy configuration error', | |
| proxyError instanceof Error | |
| ? proxyError.message | |
| : String(proxyError) | |
| ); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In packages/altair-electron/src/app/index.ts around lines 63 to 65, the await
setupProxy(settings) call can throw and currently leaves the app without a UI;
wrap the call in a try/catch, and in the catch block surface the error (e.g.,
use Electron's dialog.showErrorBox or processLogger.error with the error stack)
and then exit gracefully (app.exit(1) or process.exit(1)) so the failure is
handled instead of letting the promise reject unhandled.
| requestBuffer = Buffer.concat([requestBuffer, chunk]); | ||
| const requestStr = requestBuffer.toString('utf8'); | ||
|
|
||
| const result = this.processAndAuthenticateRequest(requestStr); | ||
| if (!result) { | ||
| // Haven't received complete headers yet, wait for more data | ||
| return; | ||
| } | ||
|
|
||
| if (!result.authenticated) { | ||
| error(`[TCP] Authentication failed for ${connectionId}`); | ||
| this.sendUnauthorized(tcpSocket); | ||
| return; | ||
| } | ||
|
|
||
| log(`[TCP] Authentication successful for ${connectionId}`); | ||
|
|
||
| // Remove the data listener now that we've authenticated | ||
| tcpSocket.off('data', onInitialData); | ||
|
|
||
| // Create UDS connection | ||
| udsSocket = net.connect(this.udsPath); | ||
|
|
||
| // Store connection pair | ||
| this.connections.set(connectionId, { tcpSocket, udsSocket }); | ||
|
|
||
| // Forward the sanitized request (with auth header stripped) to UDS | ||
| udsSocket.write(Buffer.from(result.sanitizedRequest, 'utf8')); | ||
|
|
There was a problem hiding this comment.
Preserve binary payloads when forwarding requests through the bridge
The current toString('utf8') → Buffer.from(..., 'utf8') round-trip rewrites the entire HTTP request. Any binary body (multipart uploads, compressed traffic) is corrupted before it reaches the UDS. Keep everything as buffers and only decode the headers you need for auth.
- const onInitialData = (chunk: Buffer) => {
- requestBuffer = Buffer.concat([requestBuffer, chunk]);
- const requestStr = requestBuffer.toString('utf8');
-
- const result = this.processAndAuthenticateRequest(requestStr);
+ const onInitialData = (chunk: Buffer) => {
+ requestBuffer = Buffer.concat([requestBuffer, chunk]);
+
+ const result = this.processAndAuthenticateRequest(requestBuffer);
@@
- // Forward the sanitized request (with auth header stripped) to UDS
- udsSocket.write(Buffer.from(result.sanitizedRequest, 'utf8'));
+ // Forward the sanitized request (with auth header stripped) to UDS
+ udsSocket.write(result.sanitizedRequest);
@@
- private processAndAuthenticateRequest(
- data: string
- ):
- | { authenticated: true; sanitizedRequest: string }
+const HEADER_SEPARATOR = Buffer.from('\r\n\r\n');
+
+ private processAndAuthenticateRequest(
+ data: Buffer
+ ):
+ | { authenticated: true; sanitizedRequest: Buffer }
@@
- const headersEndIndex = data.indexOf('\r\n\r\n');
+ const headersEndIndex = data.indexOf(HEADER_SEPARATOR);
if (headersEndIndex === -1) {
// Haven't received complete headers yet, wait for more data
return undefined;
}
- const headersStr = data.substring(0, headersEndIndex);
- const body = data.substring(headersEndIndex);
- const lines = headersStr.split('\r\n');
+ const headerSectionLength = headersEndIndex + HEADER_SEPARATOR.length;
+ const headersBuffer = data.subarray(0, headerSectionLength);
+ const bodyBuffer = data.subarray(headerSectionLength);
+ const lines = headersBuffer
+ .subarray(0, headersBuffer.length - HEADER_SEPARATOR.length)
+ .toString('utf8')
+ .split('\r\n');
@@
- const sanitizedRequest = sanitizedLines.join('\r\n') + body;
- return { authenticated: true, sanitizedRequest };
+ const sanitizedHeaders = `${sanitizedLines.join('\r\n')}\r\n\r\n`;
+ const sanitizedRequest = Buffer.concat([
+ Buffer.from(sanitizedHeaders, 'utf8'),
+ bodyBuffer,
+ ]);
+ return { authenticated: true, sanitizedRequest };🤖 Prompt for AI Agents
In packages/altair-electron/src/proxy/tcp-uds-bridge.ts around lines 28 to 56,
the code converts the entire request buffer to a UTF-8 string and then back to a
buffer which corrupts any binary request bodies; instead, treat the incoming
data as Buffer throughout, find the end-of-headers boundary (CRLF CRLF) in the
accumulated Buffer, only UTF-8 decode the header portion to locate and remove
the Authorization header, build a sanitized headers Buffer (re-encode only the
modified header text) and then concat that sanitized headers Buffer with the
original body Buffer (everything after the header boundary) and write that
combined Buffer to udsSocket so binary payloads remain intact.
| const server = net.createServer((tcpSocket) => { | ||
| const udsSocket = net.connect(UDS_PATH); | ||
| tcpSocket.pipe(udsSocket); | ||
| udsSocket.pipe(tcpSocket); | ||
| }); | ||
|
|
||
| server.listen(TCP_PORT, TCP_HOST, () => { | ||
| console.log(`Bridge running tcp://${TCP_HOST}:${TCP_PORT} → ${UDS_PATH}`); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling and cleanup for socket connections.
The script lacks error handling for connection failures and socket errors, which can lead to silent failures and resource leaks.
Apply this diff to add proper error handling:
const server = net.createServer((tcpSocket) => {
const udsSocket = net.connect(UDS_PATH);
+
+ udsSocket.on('error', (err) => {
+ console.error('UDS connection error:', err);
+ tcpSocket.destroy();
+ });
+
+ tcpSocket.on('error', (err) => {
+ console.error('TCP socket error:', err);
+ udsSocket.destroy();
+ });
+
tcpSocket.pipe(udsSocket);
udsSocket.pipe(tcpSocket);
+
+ tcpSocket.on('close', () => {
+ udsSocket.destroy();
+ });
+
+ udsSocket.on('close', () => {
+ tcpSocket.destroy();
+ });
});
server.listen(TCP_PORT, TCP_HOST, () => {
console.log(`Bridge running tcp://${TCP_HOST}:${TCP_PORT} → ${UDS_PATH}`);
+}).on('error', (err) => {
+ console.error('Server error:', err);
+ process.exit(1);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const server = net.createServer((tcpSocket) => { | |
| const udsSocket = net.connect(UDS_PATH); | |
| tcpSocket.pipe(udsSocket); | |
| udsSocket.pipe(tcpSocket); | |
| }); | |
| server.listen(TCP_PORT, TCP_HOST, () => { | |
| console.log(`Bridge running tcp://${TCP_HOST}:${TCP_PORT} → ${UDS_PATH}`); | |
| }); | |
| const server = net.createServer((tcpSocket) => { | |
| const udsSocket = net.connect(UDS_PATH); | |
| udsSocket.on('error', (err) => { | |
| console.error('UDS connection error:', err); | |
| tcpSocket.destroy(); | |
| }); | |
| tcpSocket.on('error', (err) => { | |
| console.error('TCP socket error:', err); | |
| udsSocket.destroy(); | |
| }); | |
| tcpSocket.pipe(udsSocket); | |
| udsSocket.pipe(tcpSocket); | |
| tcpSocket.on('close', () => { | |
| udsSocket.destroy(); | |
| }); | |
| udsSocket.on('close', () => { | |
| tcpSocket.destroy(); | |
| }); | |
| }); | |
| server.listen(TCP_PORT, TCP_HOST, () => { | |
| console.log(`Bridge running tcp://${TCP_HOST}:${TCP_PORT} → ${UDS_PATH}`); | |
| }).on('error', (err) => { | |
| console.error('Server error:', err); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In scripts/http-to-uds.ts around lines 8 to 16, the TCP↔UDS bridge lacks error
handling and cleanup for socket connections; add 'error' listeners on both
tcpSocket and udsSocket to log the error and destroy the peer socket, add
'close'/'end' handlers to unpipe and destroy the other socket to avoid leaks,
handle connection errors from net.connect (use the 'connect' and 'error' events
or pass a callback) and ensure tcpSocket is ended/destroyed if UDS connect
fails, add server.on('error') to log and exit or recover, and add process signal
handlers (SIGINT/SIGTERM) that close the server and destroy active sockets to
ensure graceful shutdown and resource cleanup.
| Script started on Sat Oct 4 16:32:49 2025 | ||
| [1m[7m%[27m[1m[0m ]2;samuel@Samuels-MBP: ~/work/altair/scripts]1;..ltair/scripts]2;samuel@Samuels-MBP: ~/work/altair/scripts]1;..ltair/scripts[0m[27m[24m[J[01;32m➜ [36mscripts[00m [01;34mgit:([31mmaster[34m) [33m✗[00m [K[?1h=[?2004h[4mp[24m[24m[1m[31mp[1m[31mn[0m[39m[1m[31mp[1m[31mn[1m[31mp[0m[39m[0m[32mp[0m[32mn[0m[32mp[32mm[39m s [4m.[24m[4m.[4m/[24m[4m/[4mh[24m[4mh[4mt[24m[4mt[4mt[24m[4mt[4mp[24m[4mp[4m-to-uds.ts[24m[1m [0m[0m [?1l>[?2004l | ||
| ]2;pnpm s ./http-to-uds.ts]1;pnpm]2;pnpm s ./http-to-uds.ts]1;pnpm[1G[0K⠙[1G[0K⠹[1G[0K⠸[1G[0K⠼[1G[0K⠴[1G[0K⠦[1G[0K⠧[1G[0K⠇[1G[0K⠏[1G[0K⠋[1G[0K⠙[1G[0K⠹[1G[0K[34mminipass[39m | ||
| minimal implementation of a PassThrough stream | ||
| Version [34m7.1.2[39m published [34m2024-05-24[39m by [34misaacs[39m | ||
| Maintainers: isaacs | ||
| Keywords: passthrough stream | ||
| [34mhttps://npm.im/minipass[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mhttp-proxy-agent[39m | ||
| An HTTP(s) proxy `http.Agent` implementation for HTTP | ||
| Version [34m7.0.2[39m published [34m2024-02-15[39m by [34mtootallnate[39m | ||
| Maintainers: tootallnate | ||
| Keywords: http proxy endpoint agent | ||
| [34mhttps://npm.im/http-proxy-agent[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34muri-js[39m | ||
| An RFC 3986/3987 compliant, scheme extendable URI/IRI parsing/validating/resolving library for JavaScript. | ||
| Version [34m4.4.1[39m published [34m2021-01-10[39m by [34mgarycourt[39m | ||
| Maintainers: garycourt | ||
| Keywords: URI IRI IDN URN UUID HTTP HTTPS WS WSS MAILTO RFC3986 RFC3987 RFC5891 RFC2616 RFC2818 RFC2141 RFC4122 RFC4291 RFC5952 RFC6068 RFC6455 RFC6874 | ||
| [34mhttps://npm.im/uri-js[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mhttp-proxy-middleware[39m | ||
| The one-liner node.js proxy middleware for connect, express, next.js and more | ||
| Version [34m3.0.5[39m published [34m2025-04-10[39m by [34mchimurai[39m | ||
| Maintainers: chimurai | ||
| Keywords: reverse proxy middleware http https connect express fastify polka next.js browser-sync gulp grunt-contrib-connect websocket ws cors | ||
| [34mhttps://npm.im/http-proxy-middleware[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mhttp-parser-js[39m | ||
| A pure JS HTTP parser for node. | ||
| Version [34m0.5.10[39m published [34m2025-04-08[39m by [34mjimbly[39m | ||
| Maintainers: creationix jimbly | ||
| Keywords: http | ||
| [34mhttps://npm.im/http-parser-js[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34magent-base[39m | ||
| Turn a function into an `http.Agent` instance | ||
| Version [34m7.1.4[39m published [34m2025-07-07[39m by [34mtootallnate[39m | ||
| Maintainers: tootallnate | ||
| Keywords: http agent base barebones https | ||
| [34mhttps://npm.im/agent-base[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mhttp-deceiver[39m | ||
| Deceive HTTP parser | ||
| Version [34m1.2.7[39m published [34m2016-05-17[39m by [34mindutny[39m | ||
| Maintainers: daviddias indutny | ||
| Keywords: http net deceive | ||
| [34mhttps://npm.im/http-deceiver[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34m@smithy/node-http-handler[39m | ||
| Provides a way to make requests | ||
| Version [34m4.3.0[39m published [34m2025-09-30[39m by [34msmithy-team[39m | ||
| Maintainers: trivikr-aws smithy-team aws-sdk-bot | ||
| [34mhttps://npm.im/@smithy/node-http-handler[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mpac-proxy-agent[39m | ||
| A PAC file proxy `http.Agent` implementation for HTTP | ||
| Version [34m7.2.0[39m published [34m2025-02-18[39m by [34mtootallnate[39m | ||
| Maintainers: tootallnate | ||
| Keywords: pac proxy agent http https socks request access | ||
| [34mhttps://npm.im/pac-proxy-agent[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mstream-http[39m | ||
| Streaming http in the browser | ||
| Version [34m3.2.0[39m published [34m2021-04-14[39m by [34mjhiesey[39m | ||
| Maintainers: feross jhiesey | ||
| Keywords: http stream streaming xhr http-browserify | ||
| [34mhttps://npm.im/stream-http[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mhttps-proxy-agent[39m | ||
| An HTTP(s) proxy `http.Agent` implementation for HTTPS | ||
| Version [34m7.0.6[39m published [34m2024-12-07[39m by [34mtootallnate[39m | ||
| Maintainers: tootallnate | ||
| Keywords: https proxy endpoint agent | ||
| [34mhttps://npm.im/https-proxy-agent[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34m@azure/core-http-compat[39m | ||
| Core HTTP Compatibility Library to bridge the gap between Core V1 & V2 packages. | ||
| Version [34m2.3.1[39m published [34m2025-09-11[39m by [34mmicrosoft1es[39m | ||
| Maintainers: azure-sdk microsoft1es | ||
| Keywords: azure cloud | ||
| [34mhttps://npm.im/@azure/core-http-compat[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mws[39m | ||
| Simple to use, blazing fast and thoroughly tested websocket client and server for Node.js | ||
| Version [34m8.18.3[39m published [34m2025-06-28[39m by [34mlpinca[39m | ||
| Maintainers: einaros v1 lpinca 3rdeden | ||
| Keywords: HyBi Push RFC-6455 WebSocket WebSockets real-time | ||
| [34mhttps://npm.im/ws[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mrequires-port[39m | ||
| Check if a protocol requires a certain port number to be added to an URL. | ||
| Version [34m1.0.0[39m published [34m2015-10-30[39m by [34m3rdeden[39m | ||
| Maintainers: v1 3rdeden | ||
| Keywords: port require http https ws wss gopher file ftp requires requried portnumber url parsing validation cows | ||
| [34mhttps://npm.im/requires-port[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mwait-on[39m | ||
| wait-on is a cross platform command line utility and Node.js API which will wait for files, ports, sockets, and http(s) resources to become available | ||
| Version [34m9.0.1[39m published [34m2025-09-20[39m by [34mjeffbski[39m | ||
| Maintainers: jeffbski | ||
| Keywords: wait delay cli files tcp ports sockets http exist ready available portable cross-platform unix linux windows win32 osx | ||
| [34mhttps://npm.im/wait-on[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mproxy-agent[39m | ||
| Maps proxy protocols to `http.Agent` implementations | ||
| Version [34m6.5.0[39m published [34m2024-12-07[39m by [34mtootallnate[39m | ||
| Maintainers: tootallnate | ||
| Keywords: http https socks agent mapping proxy | ||
| [34mhttps://npm.im/proxy-agent[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mcssom[39m | ||
| CSS Object Model implementation and CSS parser | ||
| Version [34m0.5.0[39m published [34m2021-06-20[39m by [34mnv[39m | ||
| Maintainers: nv domenic | ||
| Keywords: CSS CSSOM parser styleSheet | ||
| [34mhttps://npm.im/cssom[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34m@opentelemetry/instrumentation-http[39m | ||
| OpenTelemetry instrumentation for `node:http` and `node:https` http client and server modules | ||
| Version [34m0.205.0[39m published [34m2025-09-10[39m by [34mdyladan[39m | ||
| Maintainers: dyladan pichlermarc | ||
| Keywords: opentelemetry http nodejs tracing profiling instrumentation | ||
| [34mhttps://npm.im/@opentelemetry/instrumentation-http[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mminipass-fetch[39m | ||
| An implementation of window.fetch in Node.js using Minipass streams | ||
| Version [34m4.0.1[39m published [34m2025-02-26[39m by [34mnpm-cli-ops[39m | ||
| Maintainers: gar saquibkhan npm-cli-ops reggi hashtagchris owlstronaut | ||
| Keywords: fetch minipass node-fetch window.fetch | ||
| [34mhttps://npm.im/minipass-fetch[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[34mnormalize-url[39m | ||
| Normalize a URL | ||
| Version [34m8.1.0[39m published [34m2025-09-10[39m by [34msindresorhus[39m | ||
| Maintainers: sindresorhus | ||
| Keywords: normalize url uri address string normalization normalisation query querystring simplify strip trim canonical | ||
| [34mhttps://npm.im/normalize-url[39m | ||
|
|
||
| [1G[0K⠹[1G[0K[1m[7m%[27m[1m[0m ]2;samuel@Samuels-MBP: ~/work/altair/scripts]1;..ltair/scripts]2;samuel@Samuels-MBP: ~/work/altair/scripts]1;..ltair/scripts[0m[27m[24m[J[01;32m➜ [36mscripts[00m [01;34mgit:([31mmaster[34m) [33m✗[00m [K[?1h=[?2004h[32mpnpm[39m s [4m./http-to-uds.ts[24mc[24m [4m.[4m/[4mh[4mt[4mt[4mp[4m-[4mt[4mo[4m-[4mu[4md[4ms[4m.[4mt[4ms[24m[17D[?1l>[?2004l | ||
| ]2;pnpm sc ./http-to-uds.ts]1;pnpm]2;pnpm sc ./http-to-uds.ts]1;pnpm | ||
| > [email protected] sc /Users/samuel/work/altair/scripts | ||
| > ts-node-dev "./http-to-uds.ts" | ||
|
|
||
| [[36mINFO[0m] [30;1m16:34:27[0m ts-node-dev ver. 2.0.0 (using ts-node ver. 10.9.2, typescript ver. 5.9.3) | ||
| Bridge running tcp://127.0.0.1:8080 → /tmp/http-proxy.sock | ||
| [31mError: connect ECONNREFUSED /tmp/http-proxy.sock | ||
| at PipeConnectWrap.afterConnect [as oncomplete] (node:net:1611:16)[39m | ||
| [[31;1mERROR[0m] [30;1m16:34:58[0m Error: connect ECONNREFUSED /tmp/http-proxy.sock | ||
| ^C[1m[7m%[27m[1m[0m ]2;samuel@Samuels-MBP: ~/work/altair/scripts]1;..ltair/scripts]2;samuel@Samuels-MBP: ~/work/altair/scripts]1;..ltair/scripts[0m[27m[24m[J[01;32m➜ [36mscripts[00m [01;34mgit:([31mmaster[34m) [33m✗[00m [K[?1h=[?2004h[32mpnpm[39m sc [4m./http-to-uds.ts[24m[?1l>[?2004l | ||
| ]2;pnpm sc ./http-to-uds.ts]1;pnpm]2;pnpm sc ./http-to-uds.ts]1;pnpm | ||
| > [email protected] sc /Users/samuel/work/altair/scripts | ||
| > ts-node-dev "./http-to-uds.ts" | ||
|
|
||
| [[36mINFO[0m] [30;1m16:35:56[0m ts-node-dev ver. 2.0.0 (using ts-node ver. 10.9.2, typescript ver. 5.9.3) | ||
| Bridge running tcp://127.0.0.1:8080 → /tmp/http-proxy.sock |
There was a problem hiding this comment.
Remove terminal log file from the repository.
This file appears to be a terminal session recording/log rather than source code. Terminal logs and session recordings should not be committed to the repository as they:
- Add unnecessary clutter
- May contain sensitive information
- Are not useful for other developers
- Can become stale quickly
Please remove this file from the commit:
#!/bin/bash
# Remove the terminal log file
git rm scripts/typescript🤖 Prompt for AI Agents
In scripts/typescript around lines 1-158 there is a terminal session recording
(log) committed into source control; remove it from the repo history by deleting
the file with git rm and committing that change, then add an appropriate pattern
(e.g., a logs/ or *.log entry) to .gitignore to prevent re-adding such session
files; if you need to keep it locally, add it to .git/info/exclude instead of
committing.
| return; | ||
| } | ||
|
|
||
| const target = parse(path).host ? path : `http://${host}${path}`; |
There was a problem hiding this comment.
Address SSRF vulnerability in test script.
The code constructs target URLs from user-controlled data without validation (line 44), allowing arbitrary HTTP requests to any destination. SonarCloud correctly flagged this as a Server-Side Request Forgery (SSRF) vulnerability.
Context: Since this is a test/development script (as indicated by location in scripts/ and usage comments), this may be acceptable for local testing. However, if this code or pattern is used elsewhere in production code, it poses a significant security risk.
Recommendations:
- Add a clear warning in the file comments about the security implications
- If used for testing, consider restricting allowed destinations to a whitelist
- Ensure this script is never exposed in production
Apply this diff to add a security warning:
/* eslint-disable no-console */
/**
* A simple UDS HTTP proxy server.
* UDS -> HTTP proxy.
+ *
+ * ⚠️ WARNING: This is a test script with no destination validation.
+ * It can proxy requests to ANY destination (SSRF vulnerability).
+ * DO NOT use this code in production or expose it to untrusted clients.
+ *
* For testing purposes. For a simple test, you can use curl:
*
* curl --unix-socket /tmp/http-proxy.sock http://example.com/
* curl --unix-socket /tmp/http-proxy.sock -x http://localhost http://example.com
*/If you want to restrict destinations for safer testing, consider adding a whitelist:
const ALLOWED_HOSTS = new Set(['example.com', 'localhost', '127.0.0.1']);
// In the request handler, after parsing the target:
const parsed = parse(target);
if (parsed.hostname && !ALLOWED_HOSTS.has(parsed.hostname)) {
clientSocket.end('HTTP/1.1 403 Forbidden\r\n\r\nDestination not allowed');
return;
}Also applies to: 62-71
🤖 Prompt for AI Agents
In scripts/uds-to-http.ts around lines 44 and 62-71, the code builds target URLs
from unvalidated input which creates an SSRF risk; add a prominent security
warning comment at the top of the file stating this is a test/dev-only script
and must not be used in production, implement destination validation by parsing
the target and checking its hostname against a maintained whitelist (reject and
close the client socket with an HTTP 403 response when the hostname is not
allowed), and ensure any fallback URL construction still uses the
validated/whitelisted host; keep the whitelist configurable and document that it
must only include safe test hosts (e.g., localhost, 127.0.0.1) and that this
script should be run only in local testing environments.
| } | ||
| }); | ||
|
|
||
| clientSocket.on('error', () => {}); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve error handling for client socket.
The empty error handler silently swallows all client socket errors, making debugging difficult.
Apply this diff to add minimal error logging:
- clientSocket.on('error', () => {});
+ clientSocket.on('error', (err) => {
+ console.error('Client socket error:', err.message);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| clientSocket.on('error', () => {}); | |
| clientSocket.on('error', (err) => { | |
| console.error('Client socket error:', err.message); | |
| }); |
🤖 Prompt for AI Agents
In scripts/uds-to-http.ts around line 82, the clientSocket currently has an
empty 'error' handler which swallows all errors; replace it with a minimal
handler that accepts the error parameter, logs it (e.g., console.error('client
socket error', err) or use your logger), and ensure the socket is cleaned up
(clientSocket.destroy() or clientSocket.end()) to avoid leaking resources.
1 participant
Fixes
Checks
yarn test-buildChanges proposed in this pull request:
Summary by Sourcery
Add full support for Unix domain socket (UDS) proxies in the Electron app by extending the settings UI, implementing a TCP-to-UDS bridge with authentication, integrating it into the proxy setup lifecycle, and providing sample scripts for testing.
New Features:
Enhancements:
Chores:
Summary by CodeRabbit
New Features
Improvements
Chores