feat(caido): add Caido plugin for Titus secret scanning#291
Draft
anushkavirgaonkar wants to merge 2 commits into
Draft
feat(caido): add Caido plugin for Titus secret scanning#291anushkavirgaonkar wants to merge 2 commits into
anushkavirgaonkar wants to merge 2 commits into
Conversation
Add a Caido proxy plugin that scans HTTP traffic for secrets using the Titus detection engine via its `titus serve` NDJSON subprocess protocol, mirroring the architecture of the existing Burp Suite extension. Backend: spawns titus serve, intercepts responses via onInterceptResponse, filters non-scannable content, deduplicates findings, reports via Caido findings API. Frontend: sidebar tab with stats dashboard, findings table, and settings toggles for passive/request scanning and scope filtering. Closes: LAB-1364 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review
Critical Issues
caido/packages/frontend/src/index.ts:11importsAPIfromtitus-backend, butcaido/packages/frontend/package.json:10does not declaretitus-backendas a dependency, andcaido/packages/backend/package.json:1exposes nomain/types/exportspointing atsrc/index.tsor generated declarations. The frontend package is therefore not resolvable in a normal pnpm workspace build/typecheck.caido/packages/backend/src/scanner.ts:184stores every in-flight request in the samependingResolve/pendingRejectfields, whilecaido/packages/backend/src/index.ts:210can run multiple intercepted responses concurrently and callscanner.scan()atindex.ts:242. A second scan overwrites the first promise beforetitus serveresponds, causing hung scans and response/result mis-association under normal proxy traffic. The wrapper needs serialization or request correlation.
Security
caido/packages/backend/src/dedup.ts:85uses a 32-bit non-cryptographic hash as the only dedupe key forruleId:secretContent. Collisions can suppress distinct findings, and the cache is explicitly sized up to 50k entries, where accidental collisions are realistic. Use a collision-resistant digest or the full composite key.
Suggestions
None.
Reviewed by Codex (gpt-5.5)
There was a problem hiding this comment.
Claude Review
Critical issues
- Concurrent scans corrupt/lose responses (
scanner.ts). The NDJSON protocol is strictly serial (one request → one response line, no request IDs), and the Java reference enforces this by makingscan()/scanBatch()/close()synchronized. The TS port has a singlependingResolve/pendingRejectpair with no serialization, butsdk.events.onInterceptResponsefires concurrently andawait scanner.scan(...)yields. With two responses in flight: call B overwrites call A'spendingResolve(A's promise hangs forever / leaks), then titus's first reply resolves B with A's matches (wrong URL/secret correlation), and the second reply arrives with no pending handler and is silently dropped. Under any real proxy traffic this mis-attributes and loses findings. Needs a request queue/mutex so only onesendRequestis outstanding at a time. - In-flight scans hang on process death (
scanner.ts). Theexitanderrorhandlers (and the close path) never callpendingReject. Iftitus servedies mid-scan, the awaiting promise never settles — and there is no per-request timeout, so a stuck subprocess wedges the handler indefinitely. The Java version returns whenreadLine()yields null; the TS port has no equivalent. - Weak 32-bit dedup hash silently drops distinct secrets (
dedup.ts).computeKeyis a 32-bit string hash (the comment even says "in production you'd use SHA-256"). WithMAX_CACHE_SIZE = 50_000, birthday collisions are likely (~30%+), and a collision maps two different secrets to the same key, so the second is treated as a duplicate and never reported. For a secret scanner, dropping real findings is a correctness bug — use a full-width hash (e.g. SHA-256) or add more discriminating data to the key.
Security
- Subprocess spawn of
titus serveis local-only (searches~/.titus,~/bin,/usr/local/bin, then baretituson PATH) with no new network calls; reasonable for this tool. Note the bare-titusPATH fallback executes whatevertitusresolves to in the plugin's environment — fine in practice but worth awareness. - Findings (full
secretContent) are held in memory and rendered into the UI; the frontend escapes viaescapeHtmlbeforeinnerHTML, so no obvious XSS from scanned content.
Test coverage
- ~1000 lines of new backend TS (
scanner.ts,dedup.ts,filter.ts) ship with no tests. At minimum cover the dedup key/eviction logic and NDJSON request/response correlation — the concurrency bug above would be caught by a test issuing two overlappingscan()calls.
There was a problem hiding this comment.
Gemini Review
Critical Issues
- Concurrency Race Condition in
TitusScanner:sendRequestindiscriminately overwritesthis.pendingResolveandthis.pendingRejectwithout queuing. Because Caido invokesonInterceptResponseconcurrently for parallel HTTP requests, simultaneous scans will overwrite each other's pending promises. When responses sequentially arrive fromtitus serve, they will resolve the last requested promise with the first response's data, causing mismatched findings and leaving earlier promises to hang indefinitely (memory leak). - High Probability Deduplication Hash Collisions: In
DedupCache.computeKey, the deduplication key is calculated using a 32-bit integer DJB2-like hash (hash |= 0). With a max cache size of 50,000, the Birthday Paradox guarantees an extremely high likelihood of collisions (~25% at capacity). A collision will silently merge two distinct secrets under the same record, incorrectly dropping valid findings. Use the string value (combined) directly as the Map key instead. - Initialization Timeout Hang on Startup Failure: In
TitusScanner.initialize, theprocess.on("exit")handler correctly updates boolean state but fails to reject the pending promise if the binary exits immediately (e.g., missing shared libraries or execute permissions). This forces the user to wait for the entire 30-secondtimeoutbefore seeing an initialization error.
Security
- No security concerns flagged.
Suggestions
- Prefer
NamedGroupsoverSnippet.Matching:extractMatchContentcurrently prefers positionalGroupsand then falls back toSnippet.Matching. Since positionalGroupsare deprecated in Titus in favor ofNamedGroups(perpkg/types/match.go), this function will frequently skip toSnippet.Matching. Snippets often include surrounding context (e.g.,api_key = "foo"instead of justfoo), degrading UI presentation and deduping effectiveness. Prioritize extracting fromNamedGroupsbefore falling back to snippets. - Non-Serializable
Setin API Payload:FindingRecordincludesurls: Set<string>. When returned to the frontend via the IPC layer (sdk.api.register("getFindings", ...)),Setobjects cannot be natively serialized to JSON without a custom replacer and will arrive as empty objects ({}). Since the frontend only reads the singular.urlproperty, consider removing.urlsfrom the exported interface or converting it to an array before transmission. - Use
Bufferfor Base64 Decoding: ThedecodeBase64function relies onatob(), which converts a base64 string into a binary string rather than properly decoding UTF-8 bytes. Since the backend executes in a Node.js context (inferred fromchild_processandfsimports), preferBuffer.from(b64, 'base64').toString('utf-8')to correctly decode UTF-8 secret values and prevent mojibake.
Reviewed by Gemini (gemini-3.1-pro-preview)
![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: ef93a9f191
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+227
to
+230
| const bodyStr = | ||
| typeof responseBody === "string" | ||
| ? responseBody | ||
| : new TextDecoder().decode(responseBody as ArrayBuffer); |
There was a problem hiding this comment.
For any intercepted response with a body, response.getBody() returns Caido's Body object rather than a string or ArrayBuffer. This branch therefore calls TextDecoder.decode() on the Body object, throws, and the catch logs a scan error before scanner.scan is ever reached; the request-body path below has the same issue. Use responseBody.toText()/requestBody.toText() before filtering and scanning.
Useful? React with 👍 / 👎.
Comment on lines
+233
to
+234
| const contentType = response.getHeader("content-type") ?? undefined; | ||
| if (!filter.shouldScan(contentType, bodyStr.length)) return; |
There was a problem hiding this comment.
When a response has a Content-Type, Caido returns header values as an array, so passing that value into FastPathFilter.shouldScan makes contentType.toLowerCase() throw instead of filtering/scanning the response. After the body conversion is fixed, most normal HTTP responses will still fail here unless this uses a single header value (or joins the values) before calling the filter.
Useful? React with 👍 / 👎.
Comment on lines
+184
to
+186
| return new Promise<TitusResponse>((resolve, reject) => { | ||
| this.pendingResolve = resolve; | ||
| this.pendingReject = reject; |
There was a problem hiding this comment.
Under normal proxy traffic, two onInterceptResponse callbacks can call scanner.scan() before Titus answers the first request. Each call overwrites this single pendingResolve/pendingReject, so the first promise is left hanging and the first Titus response is delivered to the later scan (misattributing or dropping findings). Queue requests or reject/await while one is in flight before writing another NDJSON request.
Useful? React with 👍 / 👎.
WalkthroughAdds a complete Caido plugin workspace ( ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
caido/README.md (2)
65-65: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
Line 65 uses a bare code fence; it should specify
bashfor syntax highlighting and clarity.Fix fenced code block
-``` +```bash🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@caido/README.md` at line 65, The fenced code block at line 65 in caido/README.md is missing a language specifier. Change the bare opening code fence from ``` to ```bash to enable proper syntax highlighting and clarify that the code block contains bash commands.
124-124: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove redundant adverb "Only" from the Scope Only setting description.
The phrase "Only scan in-scope targets" repeats the "Only" concept from the setting name. Simplify to "Scan in-scope targets only" or "Scope filtered."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@caido/README.md` at line 124, The description for the "Scope Only" setting contains a redundant use of the word "Only" by stating "Only scan in-scope targets", which repeats the concept already present in the setting name. Locate the row with the "Scope Only" setting in the table and modify the description text from "Only scan in-scope targets" to either "Scan in-scope targets only" or "Scope filtered" to eliminate the redundancy while maintaining clarity.caido/packages/backend/src/scanner.ts (1)
84-86: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winDrain child
stderrto avoid subprocess backpressure stalls.You pipe
stderr(Line 85) but never read it. If Titus emits enough stderr output, the child can block.Suggested fix
try { this.process = spawn(titusPath, ["serve"], { stdio: ["pipe", "pipe", "pipe"], }); } catch (err) { @@ this.process.on("error", (err) => { clearTimeout(timeout); reject(new Error(`Titus process error: ${err.message}`)); }); + + this.process.stderr?.on("data", (chunk: Buffer) => { + // Keep pipe drained; optionally forward to plugin logs if desired. + void chunk; + });Also applies to: 97-100, 143-144
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@caido/packages/backend/src/scanner.ts` around lines 84 - 86, The spawn call for Titus is piping stderr but never reading from it, which can cause the child process to block when its stderr buffer fills up. After spawning the process with this.process = spawn(titusPath, ["serve"], {...}), add code to drain the stderr stream by either piping it to process.stderr or attaching a data listener to consume the output. Apply this same fix to the other two spawn calls mentioned in the comment (around lines 97-100 and 143-144) to prevent backpressure stalls in all child processes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@caido/packages/backend/src/dedup.ts`:
- Around line 85-95: The computeKey method currently uses a simple 32-bit hash
algorithm that is prone to collisions, which can cause unrelated findings to be
merged and detections to be suppressed. Replace this weak hashing approach with
a collision-resistant cryptographic hash function such as SHA-256. Update the
computeKey method to use Node.js's crypto module (or similar) to generate a
secure hash of the combined ruleId and secretContent string, ensuring the output
is deterministic and collision-resistant for proper deduplication.
In `@caido/packages/backend/src/index.ts`:
- Around line 246-255: The request body scanning in the `if (scanRequests)`
block does not apply `FastPathFilter` to check content-type and enforce max-size
constraints before scanning. This allows large or binary uploads to be scanned
unnecessarily, degrading proxy performance. Apply the same `FastPathFilter`
logic that is used for response body scanning to filter the request body based
on content-type and max-size before calling `scanner.scan()` on the request
string. Ensure the filter check happens before the length check and scanner
invocation.
In `@caido/packages/backend/src/scanner.ts`:
- Around line 179-197: The sendRequest method has two issues: first, storing a
single pendingResolve and pendingReject causes concurrent requests to overwrite
each other's handlers, misrouting responses and leaving promises hanging.
Replace the single pendingResolve/pendingReject pattern with a Map or queue that
uses request IDs or tracking tokens to store each request's resolve and reject
handlers separately. Second, the method lacks any timeout mechanism, so if the
Titus process stops responding, promises hang indefinitely. Add a timeout
promise using Promise.race that rejects after a configurable duration (e.g., 30
seconds), ensuring each request in the sendRequest method automatically fails if
no response is received within the time limit.
In `@caido/packages/frontend/src/index.ts`:
- Around line 70-75: The toggle handlers in the createToggle function calls are
performing fire-and-forget backend mutations without error handling. When
sdk.backend.setPassiveScan(enabled) or similar calls fail, the UI checkbox state
becomes out of sync with the actual backend state. Modify each toggle handler
(the callback functions passed to createToggle for "Passive Scan", and the other
toggles at the specified locations) to properly handle promise rejections from
the backend calls, either by reverting the checkbox state on failure or
displaying an error notification to the user so the UI state remains consistent
with the backend.
- Around line 205-228: The current implementation renders the entire sorted
findings array into the table every time refreshFindings is called, which causes
performance degradation with large datasets. Instead of mapping over all
findings in the sorted array and setting tbody.innerHTML with all rows at once,
limit the number of findings that are rendered by slicing the sorted array to
only include a manageable subset (such as the first 50 or 100 findings). This
pagination approach in the refreshFindings function will prevent full-table
re-renders and maintain UI responsiveness even when dealing with large finding
sets.
- Around line 97-100: The clearBtn click event listener currently calls
clearFindings() and refreshFindings() but does not refresh the statistics after
clearing findings, leaving stats stale until the next periodic update. Locate
the function responsible for refreshing statistics (likely something similar to
refreshFindings) and call it immediately after refreshFindings(sdk) in the
clearBtn event listener to ensure both the findings table and statistics totals
are updated immediately after clearing.
---
Nitpick comments:
In `@caido/packages/backend/src/scanner.ts`:
- Around line 84-86: The spawn call for Titus is piping stderr but never reading
from it, which can cause the child process to block when its stderr buffer fills
up. After spawning the process with this.process = spawn(titusPath, ["serve"],
{...}), add code to drain the stderr stream by either piping it to
process.stderr or attaching a data listener to consume the output. Apply this
same fix to the other two spawn calls mentioned in the comment (around lines
97-100 and 143-144) to prevent backpressure stalls in all child processes.
In `@caido/README.md`:
- Line 65: The fenced code block at line 65 in caido/README.md is missing a
language specifier. Change the bare opening code fence from ``` to ```bash to
enable proper syntax highlighting and clarify that the code block contains bash
commands.
- Line 124: The description for the "Scope Only" setting contains a redundant
use of the word "Only" by stating "Only scan in-scope targets", which repeats
the concept already present in the setting name. Locate the row with the "Scope
Only" setting in the table and modify the description text from "Only scan
in-scope targets" to either "Scan in-scope targets only" or "Scope filtered" to
eliminate the redundancy while maintaining clarity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6bcd8fc0-ecdd-46bd-866f-8a4858f5e528
📒 Files selected for processing (20)
caido/README.mdcaido/manifest.jsoncaido/package.jsoncaido/packages/backend/package.jsoncaido/packages/backend/src/dedup.tscaido/packages/backend/src/filter.tscaido/packages/backend/src/index.tscaido/packages/backend/src/scanner.tscaido/packages/backend/src/types.tscaido/packages/backend/tsconfig.jsoncaido/packages/backend/vite.config.tscaido/packages/frontend/package.jsoncaido/packages/frontend/src/index.tscaido/packages/frontend/src/style.csscaido/packages/frontend/tsconfig.jsoncaido/packages/frontend/vite.config.tscaido/pnpm-workspace.yamlcaido/scripts/clean.jscaido/scripts/pack.jscaido/tsconfig.json
Comment on lines
+85
to
+95
| private computeKey(secretContent: string, ruleId: string): string { | ||
| // Simple hash - in production you'd use SHA-256 | ||
| let hash = 0; | ||
| const combined = `${ruleId}:${secretContent}`; | ||
| for (let i = 0; i < combined.length; i++) { | ||
| const chr = combined.charCodeAt(i); | ||
| hash = (hash << 5) - hash + chr; | ||
| hash |= 0; | ||
| } | ||
| return hash.toString(36); | ||
| } |
There was a problem hiding this comment.
Use a collision-resistant dedup key.
Line 85 uses a 32-bit hash for ruleId + secretContent. Collisions can merge unrelated findings and suppress detections.
Suggested fix
+import { createHash } from "crypto";
import type { FindingRecord } from "./types.js";
@@
private computeKey(secretContent: string, ruleId: string): string {
- // Simple hash - in production you'd use SHA-256
- let hash = 0;
- const combined = `${ruleId}:${secretContent}`;
- for (let i = 0; i < combined.length; i++) {
- const chr = combined.charCodeAt(i);
- hash = (hash << 5) - hash + chr;
- hash |= 0;
- }
- return hash.toString(36);
+ return createHash("sha256")
+ .update(ruleId)
+ .update("\0")
+ .update(secretContent)
+ .digest("hex");
}📝 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.
Suggested change
| private computeKey(secretContent: string, ruleId: string): string { | |
| // Simple hash - in production you'd use SHA-256 | |
| let hash = 0; | |
| const combined = `${ruleId}:${secretContent}`; | |
| for (let i = 0; i < combined.length; i++) { | |
| const chr = combined.charCodeAt(i); | |
| hash = (hash << 5) - hash + chr; | |
| hash |= 0; | |
| } | |
| return hash.toString(36); | |
| } | |
| import { createHash } from "crypto"; | |
| import type { FindingRecord } from "./types.js"; | |
| private computeKey(secretContent: string, ruleId: string): string { | |
| return createHash("sha256") | |
| .update(ruleId) | |
| .update("\0") | |
| .update(secretContent) | |
| .digest("hex"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@caido/packages/backend/src/dedup.ts` around lines 85 - 95, The computeKey
method currently uses a simple 32-bit hash algorithm that is prone to
collisions, which can cause unrelated findings to be merged and detections to be
suppressed. Replace this weak hashing approach with a collision-resistant
cryptographic hash function such as SHA-256. Update the computeKey method to use
Node.js's crypto module (or similar) to generate a secure hash of the combined
ruleId and secretContent string, ensuring the output is deterministic and
collision-resistant for proper deduplication.
Comment on lines
+246
to
+255
| if (scanRequests) { | ||
| const requestBody = request.getBody(); | ||
| if (requestBody) { | ||
| const reqStr = | ||
| typeof requestBody === "string" | ||
| ? requestBody | ||
| : new TextDecoder().decode(requestBody as ArrayBuffer); | ||
| if (reqStr.length > 10) { | ||
| requestMatches = await scanner.scan(reqStr, `${url} [request]`); | ||
| } |
There was a problem hiding this comment.
Apply fast-path filtering to request-body scanning too.
At Line 246+, request scanning bypasses FastPathFilter (content-type and max-size). When enabled, large or binary uploads can be scanned and degrade proxy responsiveness.
Suggested fix
let requestMatches: TitusMatch[] = [];
if (scanRequests) {
const requestBody = request.getBody();
if (requestBody) {
- const reqStr =
- typeof requestBody === "string"
- ? requestBody
- : new TextDecoder().decode(requestBody as ArrayBuffer);
- if (reqStr.length > 10) {
- requestMatches = await scanner.scan(reqStr, `${url} [request]`);
- }
+ const requestContentType =
+ request.getHeader("content-type") ?? undefined;
+ const requestLength =
+ typeof requestBody === "string"
+ ? requestBody.length
+ : (requestBody as ArrayBuffer).byteLength;
+
+ if (filter.shouldScan(requestContentType, requestLength)) {
+ const reqStr =
+ typeof requestBody === "string"
+ ? requestBody
+ : new TextDecoder().decode(requestBody as ArrayBuffer);
+ requestMatches = await scanner.scan(reqStr, `${url} [request]`);
+ }
}
}📝 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.
Suggested change
| if (scanRequests) { | |
| const requestBody = request.getBody(); | |
| if (requestBody) { | |
| const reqStr = | |
| typeof requestBody === "string" | |
| ? requestBody | |
| : new TextDecoder().decode(requestBody as ArrayBuffer); | |
| if (reqStr.length > 10) { | |
| requestMatches = await scanner.scan(reqStr, `${url} [request]`); | |
| } | |
| if (scanRequests) { | |
| const requestBody = request.getBody(); | |
| if (requestBody) { | |
| const requestContentType = | |
| request.getHeader("content-type") ?? undefined; | |
| const requestLength = | |
| typeof requestBody === "string" | |
| ? requestBody.length | |
| : (requestBody as ArrayBuffer).byteLength; | |
| if (filter.shouldScan(requestContentType, requestLength)) { | |
| const reqStr = | |
| typeof requestBody === "string" | |
| ? requestBody | |
| : new TextDecoder().decode(requestBody as ArrayBuffer); | |
| requestMatches = await scanner.scan(reqStr, `${url} [request]`); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@caido/packages/backend/src/index.ts` around lines 246 - 255, The request body
scanning in the `if (scanRequests)` block does not apply `FastPathFilter` to
check content-type and enforce max-size constraints before scanning. This allows
large or binary uploads to be scanned unnecessarily, degrading proxy
performance. Apply the same `FastPathFilter` logic that is used for response
body scanning to filter the request body based on content-type and max-size
before calling `scanner.scan()` on the request string. Ensure the filter check
happens before the length check and scanner invocation.
Comment on lines
+179
to
+197
| private async sendRequest(request: TitusRequest): Promise<TitusResponse> { | ||
| if (!this.initialized || !this.process?.stdin) { | ||
| throw new Error("Scanner not initialized"); | ||
| } | ||
|
|
||
| return new Promise<TitusResponse>((resolve, reject) => { | ||
| this.pendingResolve = resolve; | ||
| this.pendingReject = reject; | ||
|
|
||
| const json = JSON.stringify(request) + "\n"; | ||
| this.process!.stdin!.write(json, "utf-8", (err) => { | ||
| if (err) { | ||
| this.pendingResolve = null; | ||
| this.pendingReject = null; | ||
| reject(new Error(`Failed to write to titus: ${err.message}`)); | ||
| } | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Serialize request/response handling and enforce a request timeout.
Line 185 overwrites pendingResolve/pendingReject per call. With concurrent scans (reachable from onInterceptResponse), responses can be misrouted and promises can hang. Also, this path has no timeout if Titus stops responding.
Suggested fix
+const REQUEST_TIMEOUT_MS = 15_000;
+
export class TitusScanner {
private process: ChildProcess | null = null;
@@
private pendingResolve: ((resp: TitusResponse) => void) | null = null;
private pendingReject: ((err: Error) => void) | null = null;
+ private requestChain: Promise<void> = Promise.resolve();
@@
private async sendRequest(request: TitusRequest): Promise<TitusResponse> {
if (!this.initialized || !this.process?.stdin) {
throw new Error("Scanner not initialized");
}
-
- return new Promise<TitusResponse>((resolve, reject) => {
- this.pendingResolve = resolve;
- this.pendingReject = reject;
+ const run = () =>
+ new Promise<TitusResponse>((resolve, reject) => {
+ if (this.pendingResolve || this.pendingReject) {
+ reject(new Error("Scanner request already in flight"));
+ return;
+ }
+
+ const timeout = setTimeout(() => {
+ this.pendingResolve = null;
+ this.pendingReject = null;
+ reject(new Error("Timeout waiting for titus response"));
+ }, REQUEST_TIMEOUT_MS);
+
+ this.pendingResolve = (resp) => {
+ clearTimeout(timeout);
+ resolve(resp);
+ };
+ this.pendingReject = (err) => {
+ clearTimeout(timeout);
+ reject(err);
+ };
- const json = JSON.stringify(request) + "\n";
- this.process!.stdin!.write(json, "utf-8", (err) => {
- if (err) {
- this.pendingResolve = null;
- this.pendingReject = null;
- reject(new Error(`Failed to write to titus: ${err.message}`));
- }
- });
- });
+ const json = JSON.stringify(request) + "\n";
+ this.process!.stdin!.write(json, "utf-8", (err) => {
+ if (err) {
+ clearTimeout(timeout);
+ this.pendingResolve = null;
+ this.pendingReject = null;
+ reject(new Error(`Failed to write to titus: ${err.message}`));
+ }
+ });
+ });
+
+ const queued = this.requestChain.then(run, run);
+ this.requestChain = queued.then(() => undefined, () => undefined);
+ return queued;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@caido/packages/backend/src/scanner.ts` around lines 179 - 197, The
sendRequest method has two issues: first, storing a single pendingResolve and
pendingReject causes concurrent requests to overwrite each other's handlers,
misrouting responses and leaving promises hanging. Replace the single
pendingResolve/pendingReject pattern with a Map or queue that uses request IDs
or tracking tokens to store each request's resolve and reject handlers
separately. Second, the method lacks any timeout mechanism, so if the Titus
process stops responding, promises hang indefinitely. Add a timeout promise
using Promise.race that rejects after a configurable duration (e.g., 30
seconds), ensuring each request in the sendRequest method automatically fails if
no response is received within the time limit.
Comment on lines
+70
to
+75
| const passiveToggle = createToggle( | ||
| "Passive Scan", | ||
| "titus-passive", | ||
| true, | ||
| (enabled) => sdk.backend.setPassiveScan(enabled) | ||
| ); |
There was a problem hiding this comment.
Handle backend mutation failures in toggle handlers.
At Line 154, toggle updates are fire-and-forget. If a backend call fails, the checkbox remains flipped and the UI state diverges from backend state.
Suggested fix
function createToggle(
label: string,
id: string,
defaultChecked: boolean,
- onChange: (enabled: boolean) => void
+ onChange: (enabled: boolean) => void | Promise<void>
): HTMLElement {
@@
- input.addEventListener("change", () => onChange(input.checked));
+ input.addEventListener("change", async () => {
+ const next = input.checked;
+ try {
+ await Promise.resolve(onChange(next));
+ } catch {
+ input.checked = !next;
+ }
+ });Also applies to: 78-83, 86-91, 154-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@caido/packages/frontend/src/index.ts` around lines 70 - 75, The toggle
handlers in the createToggle function calls are performing fire-and-forget
backend mutations without error handling. When
sdk.backend.setPassiveScan(enabled) or similar calls fail, the UI checkbox state
becomes out of sync with the actual backend state. Modify each toggle handler
(the callback functions passed to createToggle for "Passive Scan", and the other
toggles at the specified locations) to properly handle promise rejections from
the backend calls, either by reverting the checkbox state on failure or
displaying an error notification to the user so the UI state remains consistent
with the backend.
Comment on lines
+97
to
+100
| clearBtn.addEventListener("click", async () => { | ||
| await sdk.backend.clearFindings(); | ||
| refreshFindings(sdk); | ||
| }); |
There was a problem hiding this comment.
Refresh stats immediately after clearing findings.
Line 99 refreshes only the table. clearFindings also resets totals, so stats remain stale until the next periodic refresh.
Suggested fix
clearBtn.addEventListener("click", async () => {
await sdk.backend.clearFindings();
- refreshFindings(sdk);
+ await Promise.all([refreshStats(sdk), refreshFindings(sdk)]);
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@caido/packages/frontend/src/index.ts` around lines 97 - 100, The clearBtn
click event listener currently calls clearFindings() and refreshFindings() but
does not refresh the statistics after clearing findings, leaving stats stale
until the next periodic update. Locate the function responsible for refreshing
statistics (likely something similar to refreshFindings) and call it immediately
after refreshFindings(sdk) in the clearBtn event listener to ensure both the
findings table and statistics totals are updated immediately after clearing.
Comment on lines
+205
to
+228
| const sorted = [...findings].sort( | ||
| (a, b) => | ||
| new Date(b.firstSeen).getTime() - new Date(a.firstSeen).getTime() | ||
| ); | ||
|
|
||
| tbody.innerHTML = sorted | ||
| .map((f) => { | ||
| const severity = getSeverityFromRuleId(f.ruleId); | ||
| const color = SEVERITY_COLORS[severity] ?? SEVERITY_COLORS.info; | ||
| const time = new Date(f.firstSeen).toLocaleTimeString(); | ||
|
|
||
| return ` | ||
| <tr> | ||
| <td><span class="titus-severity" style="background:${color}">${severity.toUpperCase()}</span></td> | ||
| <td class="titus-rule" title="${escapeHtml(f.ruleId)}">${escapeHtml(f.ruleName)}</td> | ||
| <td class="titus-secret"><code>${escapeHtml(f.secretPreview)}</code></td> | ||
| <td>${escapeHtml(f.host)}</td> | ||
| <td class="titus-url" title="${escapeHtml(f.url)}">${escapeHtml(truncate(f.url, 60))}</td> | ||
| <td>${f.occurrenceCount}</td> | ||
| <td>${time}</td> | ||
| </tr> | ||
| `; | ||
| }) | ||
| .join(""); |
There was a problem hiding this comment.
Avoid full-table re-render on every newFinding event.
Line 296 triggers a full findings refresh per event, and Lines 205-228 sort/rebuild the entire table each time. With large finding sets, this will degrade UI responsiveness.
Suggested direction
+let findingsRefreshTimer: number | undefined;
+
+function scheduleFindingsRefresh(sdk: SDK): void {
+ if (findingsRefreshTimer) return;
+ findingsRefreshTimer = window.setTimeout(async () => {
+ findingsRefreshTimer = undefined;
+ await refreshFindings(sdk);
+ }, 250);
+}
@@
- sdk.backend.onEvent("newFinding" as any, () => {
- refreshStats(sdk);
- refreshFindings(sdk);
- });
+ sdk.backend.onEvent("newFinding" as any, () => {
+ refreshStats(sdk);
+ scheduleFindingsRefresh(sdk);
+ });Then add pagination/row limits in refreshFindings to avoid rendering all rows at once.
Also applies to: 296-299
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 209-227: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tbody.innerHTML = sorted
.map((f) => {
const severity = getSeverityFromRuleId(f.ruleId);
const color = SEVERITY_COLORS[severity] ?? SEVERITY_COLORS.info;
const time = new Date(f.firstSeen).toLocaleTimeString();
return `
<tr>
<td><span class="titus-severity" style="background:${color}">${severity.toUpperCase()}</span></td>
<td class="titus-rule" title="${escapeHtml(f.ruleId)}">${escapeHtml(f.ruleName)}</td>
<td class="titus-secret"><code>${escapeHtml(f.secretPreview)}</code></td>
<td>${escapeHtml(f.host)}</td>
<td class="titus-url" title="${escapeHtml(f.url)}">${escapeHtml(truncate(f.url, 60))}</td>
<td>${f.occurrenceCount}</td>
<td>${time}</td>
</tr>
`;
})
.join("")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(dom-content-modification)
[warning] 209-227: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: tbody.innerHTML = sorted
.map((f) => {
const severity = getSeverityFromRuleId(f.ruleId);
const color = SEVERITY_COLORS[severity] ?? SEVERITY_COLORS.info;
const time = new Date(f.firstSeen).toLocaleTimeString();
return `
<tr>
<td><span class="titus-severity" style="background:${color}">${severity.toUpperCase()}</span></td>
<td class="titus-rule" title="${escapeHtml(f.ruleId)}">${escapeHtml(f.ruleName)}</td>
<td class="titus-secret"><code>${escapeHtml(f.secretPreview)}</code></td>
<td>${escapeHtml(f.host)}</td>
<td class="titus-url" title="${escapeHtml(f.url)}">${escapeHtml(truncate(f.url, 60))}</td>
<td>${f.occurrenceCount}</td>
<td>${time}</td>
</tr>
`;
})
.join("")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(unsafe-html-content-assignment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@caido/packages/frontend/src/index.ts` around lines 205 - 228, The current
implementation renders the entire sorted findings array into the table every
time refreshFindings is called, which causes performance degradation with large
datasets. Instead of mapping over all findings in the sorted array and setting
tbody.innerHTML with all rows at once, limit the number of findings that are
rendered by slicing the sorted array to only include a manageable subset (such
as the first 50 or 100 findings). This pagination approach in the
refreshFindings function will prevent full-table re-renders and maintain UI
responsiveness even when dealing with large finding sets.
…ialization - Serialize scanner requests with queue to prevent concurrent response corruption - Add per-request timeout and reject pending promises on process death - Replace weak 32-bit dedup hash with full composite string key - Use Buffer.from for base64 decoding, prefer NamedGroups over Groups - Apply FastPathFilter to request body scanning - Fix Set<string> -> string[] for IPC serialization of finding URLs - Add backend package exports and frontend workspace dependency - Refresh stats on clear, limit table render to 100 rows - Drain child process stderr to prevent backpressure stalls Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
caido/) that scans HTTP traffic for secrets using the Titus detection engine, mirroring the Burp Suite extension architecturetitus servesubprocess and communicates via NDJSON, intercepts HTTP responses, filters non-scannable content, deduplicates findings, and reports via Caido's findings APICloses LAB-1364
Architecture
Same subprocess approach as the Burp extension -- spawns
titus serveonce, reuses for all scans. No rules ported to JS; full 444+ rule engine via the native binary.Files
caido/packages/backend/src/index.tscaido/packages/backend/src/scanner.tsTitusScanner: subprocess managementcaido/packages/backend/src/filter.tsFastPathFilter: content-type/extension filteringcaido/packages/backend/src/dedup.tsDedupCache: finding deduplicationcaido/packages/backend/src/types.tscaido/packages/frontend/src/index.tscaido/packages/frontend/src/style.cssTest plan
titusbinary, runpnpm install && pnpm buildincaido/plugin_package.zipin Caido Settings > Plugins🤖 Generated with Claude Code