Integrating TruffleHog secret detection patterns - #16
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded a batch-search feature plus supporting data, tests, and tooling. Introduced a new Pinia store and dialog, a large secret-patterns dataset and test suite, multiple frontend UI changes, backend/utility typing and null->undefined normalization, ESLint/Vitest configs, and package.json ESM and dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (UI)"
participant UI as "Frontend Components"
participant Store as "BatchSearchStore"
participant Repo as "Grep Repository"
participant SDK as "Frontend SDK"
participant Backend as "Backend API / GrepService"
rect rgba(135,206,250,0.5)
User->>UI: Click "Search All Secrets"
UI->>Store: startBatchSearch(options)
Store->>UI: show warning dialog
User->>UI: Confirm start
UI->>Store: confirmAndStart()
end
rect rgba(144,238,144,0.5)
Store->>Store: iterate SECRET_PATTERNS sequentially
Store->>Repo: searchGrepRequests(pattern, options)
Repo->>SDK: call backend API (search request)
SDK->>Backend: HTTP / IPC request
Backend->>Backend: execute grep, return matches
Backend-->>SDK: results
SDK-->>Repo: results
Repo-->>Store: results (with cancelled flag)
Store->>Store: tag matches with pattern category
Store->>UI: update results view/state
end
rect rgba(255,182,193,0.5)
alt user cancels
User->>UI: Cancel
UI->>Store: cancelSearch()
Store->>Repo: stopGrep()
Repo->>SDK: stop request
SDK->>Backend: stop
Backend-->>SDK: stopped
SDK-->>Repo: stopped
Repo-->>Store: cancelled ack
Store->>UI: show cancellation toast
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/src/services/storage.ts (1)
13-24:⚠️ Potential issue | 🟠 MajorDon’t silently suppress storage directory initialization failures.
Line 22 currently swallows all errors, which can hide real filesystem failures and make later errors harder to diagnose.
💡 Proposed fix
constructor(sdk: CaidoBackendSDK) { this.sdk = sdk; this.regexesDir = path.join(this.sdk.meta.path(), "regexes"); - this.ensureRegexesDirectory(); } private async ensureRegexesDirectory(): Promise<void> { - try { - await mkdir(this.regexesDir, { recursive: true }); - } catch { - // Directory might already exist - } + await mkdir(this.regexesDir, { recursive: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/services/storage.ts` around lines 13 - 24, The ensureRegexesDirectory method is swallowing all errors which hides real filesystem failures; update ensureRegexesDirectory (used from the constructor) to handle errors explicitly by catching the error into a variable and either rethrowing or logging it with context (including this.regexesDir and the caught error) instead of an empty catch block; ensure the chosen behavior surfaces initialization failures to callers (eg. rethrow after logging) so the constructor/SDK initialization won't silently proceed on real disk errors.
🧹 Nitpick comments (4)
packages/frontend/src/components/results/None.vue (1)
19-29: Optional: extract repeated “no results” condition into a computed flag.The same predicate appears twice, which makes future tweaks easier to miss.
♻️ Suggested refactor
<script setup lang="ts"> import Card from "primevue/card"; +import { computed } from "vue"; import { useGrepStore } from "@/stores"; const store = useGrepStore(); +const hasNoResults = computed( + () => !!store.pattern.trim() && store.results.searchResults?.length === 0, +); </script>- <i - v-if=" - store.pattern.trim() && store.results.searchResults?.length === 0 - " - class="fas fa-search-minus text-gray-400 text-3xl" - ></i> + <i v-if="hasNoResults" class="fas fa-search-minus text-gray-400 text-3xl"></i> <i v-else class="fas fa-search text-gray-400 text-3xl"></i> - <div - v-if=" - store.pattern.trim() && store.results.searchResults?.length === 0 - " - class="text-gray-400 text-md" - > + <div v-if="hasNoResults" class="text-gray-400 text-md"> No results found </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/results/None.vue` around lines 19 - 29, The template repeats the predicate store.pattern.trim() && store.results.searchResults?.length === 0 twice; add a computed property (e.g., hasNoResults or isNoResults) in the None.vue component that returns that boolean and replace both occurrences in the template with this computed flag to centralize the logic and make future changes simpler; update references to use the new computed name and ensure it imports/uses the same store instance as the template..mise.toml (1)
8-8: Use a deterministic install command in validation.
pnpm ican mask lockfile drift. Prefer a frozen-lockfile install for reproducible validation runs.♻️ Proposed change
[tasks.validate] description = "Run all checks: install, lint, typecheck, test, build" run = """ -pnpm i +pnpm install --frozen-lockfile pnpm lint pnpm typecheck pnpm test pnpm build """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.mise.toml at line 8, The validation currently runs the non-deterministic command "pnpm i" in .mise.toml; replace it with a frozen-lockfile install (e.g., "pnpm install --frozen-lockfile" or the equivalent CI-safe command for your pnpm version) so the validation fails on lockfile drift and becomes reproducible; update the command string in .mise.toml to the frozen-lockfile variant and ensure any CI/validation runner that references the "pnpm i" entry now uses that updated command.packages/frontend/src/components/search/TransformScriptDialog.vue (1)
24-28: Minor: Avoid callingtrim()twice.
localScript.value.trim()is evaluated twice when the script is non-empty. Consider storing the trimmed value first.♻️ Proposed fix
const save = () => { - grepStore.options.transformScript = - localScript.value.trim() === "" ? undefined : localScript.value.trim(); + const trimmed = localScript.value.trim(); + grepStore.options.transformScript = trimmed === "" ? undefined : trimmed; visible.value = false; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/search/TransformScriptDialog.vue` around lines 24 - 28, In save(), avoid calling localScript.value.trim() twice: compute a single const trimmed = localScript.value.trim() and then set grepStore.options.transformScript = trimmed === "" ? undefined : trimmed, finally set visible.value = false; this updates the function save and uses the variables localScript.value, grepStore.options.transformScript, and visible.value.packages/frontend/src/components/search/patterns/PatternsList.vue (1)
38-47: Prefer a stable id for custom-pattern actions.Edit/delete are keyed off
pattern.name, not a stable identifier. If custom names ever collide, both actions will resolve the first match, and the delete path can fall back to"". Thread the custom pattern id throughDisplayPatternand use that here directly.Also applies to: 155-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/search/patterns/PatternsList.vue` around lines 38 - 47, The edit/delete flows currently look up custom patterns by name (getCustomPatternById and editCustomPattern) which is fragile — change these to use the pattern's stable id: thread the custom pattern id through DisplayPattern, update the functions (e.g., getCustomPatternById -> getCustomPatternById(id: string) to find by p.id, and editCustomPattern to accept id and call patternsStore.openCustomRegexDialog(foundPattern)), and update the corresponding delete path (the code around the other occurrence at lines 155-166) to use the id instead of name so actions always target the exact custom pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eslint.config.js`:
- Line 22: Add a final newline at the end of the file so the trailing "];" line
ends with a newline to satisfy Prettier/formatting checks; locate the closing
array/semicolon token (the "];" line in eslint.config.js) and ensure the file
ends with a single newline character.
In `@packages/backend/src/services/grep.ts`:
- Line 152: Replace the native RegExp instantiation with the RE2 implementation:
import RE2 from 're2' at the top of packages/backend/src/services/grep.ts and
change the line that creates regex (currently const regex = new RegExp(pattern,
"is")) to use new RE2(pattern, "is") so user-supplied patterns are executed by
re2 (polynomial-time) instead of the native engine; keep the same variable name
(regex) and types where used so the rest of the grep logic (pattern matching
over request/response bodies) continues to work.
In `@packages/frontend/src/components/results/Results.vue`:
- Around line 170-175: The UI flips store.status.isSearching and
store.results.cancelled before the async stopGrep() completes, causing the UI to
show a stopped scan even if the backend stop fails; change stopSearch so it
awaits stopGrep() (or handles its Promise) and only set store.status.isSearching
= false and store.results.cancelled = true after stopGrep() resolves
successfully, and on rejection keep the searching state and surface the error
(so the Stop button remains available for retry); update the stopSearch function
to await stopGrep() and move the state mutations into the success path.
In `@packages/frontend/src/components/search/batch/BatchSearchDialog.vue`:
- Around line 12-18: The Dialog close paths must route through
batchSearchStore.closeWarningDialog so pendingOptions is always cleared; update
the Dialog in BatchSearchDialog.vue to disable built-in closing and bind the
hide event by setting :closable="false" and :close-on-escape="false" and adding
`@hide`="batchSearchStore.closeWarningDialog" (the v-model can remain bound to
batchSearchStore.showWarningDialog) so any user close (button/escape/other)
calls closeWarningDialog() and keeps startBatchSearch() consistent.
In `@packages/frontend/src/components/search/Form.vue`:
- Around line 33-35: handleSearch currently unconditionally calls
batchSearchStore.resetBatchState() and grepStore.searchGrepRequests(), which
allows a normal search to interrupt a running batch (and vice versa) because
both mutate the same grepStore; update handleSearch to first check a running
flag (e.g., grepStore.isRunning or batchSearchStore.isBatchRunning) and return
or disable starting a normal search if a batch run is active, and likewise
modify the batch-start path (the batch button handler referenced around lines
64-70) to check the same running flag and avoid calling
batchSearchStore.resetBatchState() or grepStore.searchGrepRequests() when the
other flow is active; use a single canonical running/mode flag on grepStore (or
batchSearchStore) to coordinate and prevent concurrent starts.
In `@packages/frontend/src/components/search/Search.vue`:
- Around line 8-12: The InputText for the search pattern needs to be disabled
during searches because handlePatternInput currently allows keystrokes to clear
batch search progress; update the InputText element to bind its disabled state
to the combined search statuses (use :disabled="grepStore.status.isSearching ||
batchSearchStore.status.isSearching") and leave handlePatternInput (which sets
grepStore.pattern and clears grepStore.currentPatternName) unchanged so user
input cannot interrupt in-progress searches.
In `@packages/frontend/src/data/secret-patterns.test.ts`:
- Around line 9-13: The testMatch helper ignores each pattern's configured
matchGroups and always prefers capture group 1; update testMatch to consult the
SecretPattern.matchGroups (e.g., use the first configured group index) when
selecting the returned capture: compute an index like const groupIndex =
Array.isArray(pattern.matchGroups) && pattern.matchGroups.length ?
pattern.matchGroups[0] : 1, validate it is a number within match.length, then
return match?.[groupIndex] ?? match?.[0] (falling back to the whole match if the
group is missing).
In `@packages/frontend/src/data/secret-patterns.ts`:
- Around line 166-168: The "JFrog Artifactory Key" detector (pattern
"\\b([a-zA-Z0-9]{64,73})\\b") and several other entries use overly broad,
context-free patterns (bare 64–73 alnum tokens, semver-like \d+\.\d+\.\d+, 44–80
char tokens, 6-digit numbers) that produce many false positives; update those
entries to either remove them or tighten them by requiring provider-specific
context/prefixes (e.g., repository/hostname, header names, known token prefixes)
or replace with stricter regexes that include surrounding keywords, or drop the
entries entirely—apply this change to the "JFrog Artifactory Key" entry and the
other context-free detectors referenced in the file so that only
provider-specific or contextual patterns remain.
In `@packages/frontend/src/stores/batchSearchStore.ts`:
- Around line 27-29: matchCategoryMap is keyed only by match.value which causes
collisions across requests and overlapping patterns; change the map to use a
stable per-match key (e.g., a generated stableMatchId or a composite key like
`${requestId}:${matchIndex}` or an inherent match.id) wherever entries are
created/updated (see matchCategoryMap usage and the code that populates it
around the existing mapping sites and the other occurrences noted). Update the
places that read/write this map (including the logic that sets and reads
selectedResultCategory) to use this stable key so categories are deterministic
across requests and overlaps, and ensure any helpers that produce keys are
consistently used in the three locations currently using match.value as the key.
- Around line 148-153: The cancelSearch() function currently flips
status.cancelled, grepStore.status.isSearching, and grepStore.results.cancelled
before calling grepRepository.stopGrep(); change this to await the asynchronous
stop operation first (await grepRepository.stopGrep()), and only set
grepStore.status.isSearching = false and grepStore.results.cancelled = true
after stopGrep resolves; also keep status.cancelled set immediately (or set it
before awaiting) so intent is recorded, and add a catch around await
grepRepository.stopGrep() to restore/adjust UI state
(grepStore.status.isSearching) or surface an error if stopping fails.
In `@packages/frontend/src/stores/grepStore.ts`:
- Around line 43-45: The code clears currentPatternName.value at the start of a
search which wipes out the label set by patternsStore; remove the line that sets
currentPatternName.value = "" so pattern-driven searches retain their label, but
keep the other initializations (results.searchResults = undefined and
status.isSearching = true) intact; locate and edit the search-starting code
where currentPatternName, results.searchResults, and status.isSearching are set
and delete only the currentPatternName reset.
- Around line 98-107: The truncation logic incorrectly treats exactly 25,000
results as truncated; update the check in the block that assigns
results.searchResults (the newResults/truncatedResults handling) to only treat
cases where newResults.length > 25000 as truncated, then slice to 25,000 and
append the synthetic warning row; ensure the condition uses > 25000 (not >=
25000) so an exact 25,000-result set is left unchanged and does not receive the
warning entry.
---
Outside diff comments:
In `@packages/backend/src/services/storage.ts`:
- Around line 13-24: The ensureRegexesDirectory method is swallowing all errors
which hides real filesystem failures; update ensureRegexesDirectory (used from
the constructor) to handle errors explicitly by catching the error into a
variable and either rethrowing or logging it with context (including
this.regexesDir and the caught error) instead of an empty catch block; ensure
the chosen behavior surfaces initialization failures to callers (eg. rethrow
after logging) so the constructor/SDK initialization won't silently proceed on
real disk errors.
---
Nitpick comments:
In @.mise.toml:
- Line 8: The validation currently runs the non-deterministic command "pnpm i"
in .mise.toml; replace it with a frozen-lockfile install (e.g., "pnpm install
--frozen-lockfile" or the equivalent CI-safe command for your pnpm version) so
the validation fails on lockfile drift and becomes reproducible; update the
command string in .mise.toml to the frozen-lockfile variant and ensure any
CI/validation runner that references the "pnpm i" entry now uses that updated
command.
In `@packages/frontend/src/components/results/None.vue`:
- Around line 19-29: The template repeats the predicate store.pattern.trim() &&
store.results.searchResults?.length === 0 twice; add a computed property (e.g.,
hasNoResults or isNoResults) in the None.vue component that returns that boolean
and replace both occurrences in the template with this computed flag to
centralize the logic and make future changes simpler; update references to use
the new computed name and ensure it imports/uses the same store instance as the
template.
In `@packages/frontend/src/components/search/patterns/PatternsList.vue`:
- Around line 38-47: The edit/delete flows currently look up custom patterns by
name (getCustomPatternById and editCustomPattern) which is fragile — change
these to use the pattern's stable id: thread the custom pattern id through
DisplayPattern, update the functions (e.g., getCustomPatternById ->
getCustomPatternById(id: string) to find by p.id, and editCustomPattern to
accept id and call patternsStore.openCustomRegexDialog(foundPattern)), and
update the corresponding delete path (the code around the other occurrence at
lines 155-166) to use the id instead of name so actions always target the exact
custom pattern.
In `@packages/frontend/src/components/search/TransformScriptDialog.vue`:
- Around line 24-28: In save(), avoid calling localScript.value.trim() twice:
compute a single const trimmed = localScript.value.trim() and then set
grepStore.options.transformScript = trimmed === "" ? undefined : trimmed,
finally set visible.value = false; this updates the function save and uses the
variables localScript.value, grepStore.options.transformScript, and
visible.value.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e922e980-0a33-453b-8154-9a555aa1583b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
.mise.tomlcaido.config.tseslint.config.jspackage.jsonpackages/backend/src/api/index.tspackages/backend/src/index.tspackages/backend/src/services/grep.test.tspackages/backend/src/services/grep.tspackages/backend/src/services/storage.tspackages/backend/src/types.tspackages/backend/src/utils/grep.tspackages/backend/src/validation/schemas.tspackages/frontend/package.jsonpackages/frontend/src/components/guide/Container.vuepackages/frontend/src/components/index.tspackages/frontend/src/components/results/Container.vuepackages/frontend/src/components/results/MatchViewer.vuepackages/frontend/src/components/results/None.vuepackages/frontend/src/components/results/Results.vuepackages/frontend/src/components/results/Searching.vuepackages/frontend/src/components/search/Container.vuepackages/frontend/src/components/search/Form.vuepackages/frontend/src/components/search/Options.vuepackages/frontend/src/components/search/Search.vuepackages/frontend/src/components/search/TransformScriptDialog.vuepackages/frontend/src/components/search/ai-dialog/AIDialog.vuepackages/frontend/src/components/search/ai-dialog/Container.vuepackages/frontend/src/components/search/batch/BatchSearchDialog.vuepackages/frontend/src/components/search/patterns/Container.vuepackages/frontend/src/components/search/patterns/CustomRegexDialog.vuepackages/frontend/src/components/search/patterns/PatternsList.vuepackages/frontend/src/data/secret-patterns.test.tspackages/frontend/src/data/secret-patterns.tspackages/frontend/src/index.tspackages/frontend/src/plugins/sdk.tspackages/frontend/src/repositories/customRegex.tspackages/frontend/src/repositories/grep.tspackages/frontend/src/stores/aiStore.tspackages/frontend/src/stores/batchSearchStore.tspackages/frontend/src/stores/grepStore.tspackages/frontend/src/stores/index.tspackages/frontend/src/stores/patternsStore.tspackages/frontend/src/types.tspackages/frontend/src/utils/ai.tspackages/frontend/src/utils/clipboard.tspackages/frontend/src/views/App.vuepackages/frontend/vitest.config.tspackages/shared/src/results.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/frontend/src/data/secret-patterns.test.ts`:
- Line 619: The test shows only the prefix because the Shopify regex
"\\b(shppa_|shpat_)([0-9A-Fa-f]{32})\\b" captures the prefix in group1 and the
hex body in group2; update the pattern in secret-patterns (the "Shopify Access
Token" regex) so the entire token is captured as a single group (e.g., wrap the
prefix+hex in one capturing group) or adjust the extraction logic to join groups
1+2; then update the test expectation to the full token (e.g., "token=shpat_"+
"a".repeat(32)) if you choose to change the test instead of the regex.
In `@packages/frontend/src/data/secret-patterns.ts`:
- Around line 875-879: There are two secret-pattern entries with the same name
"Terraform Cloud Token"; locate the object with name "Terraform Cloud Token",
pattern "\\btfp_[a-zA-Z0-9_]{40,59}\\b" and category "Other" and rename it to a
unique display name (for example "Terraform Cloud Team Token") or delete this
duplicate entry so the UI header store.currentPatternName and category filter
are unambiguous; ensure the remaining entry names are unique across categories.
- Around line 192-196: The regex for the "RubyGems API Key" secret entry uses
the character class [a-zA0-9], which accidentally omits uppercase letters B–Z;
update the pattern in the object named "RubyGems API Key" to include uppercase
A–Z as well (use a character class that contains both A-Z and a-z plus digits)
so RubyGems keys with uppercase letters are detected, then run the
secret-patterns tests to confirm detection.
- Around line 649-653: The regex for "Shopify Access Token" currently captures
the prefix and body in separate groups which, combined with the default
matchGroups behavior in testMatch and batchSearchStore, yields only the prefix;
change the pattern in secret-patterns.ts so the entire token is captured in a
single group (e.g., make the prefix + body one capture and any internal
alternation non-capturing), or alternatively set explicit matchGroups for this
pattern to return the full match; also update the test expectation in
secret-patterns.test.ts to assert the full token instead of just the prefix.
- Around line 711-720: The regexes for "Session Key" (name: "Session Key",
pattern: "(?:[^A-Za-z0-9+/]|\\A)...\\z") and the Azure DevOps Token pattern use
Go-style anchors `\A`/`\z` which are treated as literal characters in JS; update
the "Session Key" pattern to use JS lookarounds like `(?<![A-Za-z0-9+/])` for
the left boundary and `(?![A-Za-z0-9+/=])` for the right boundary, and update
the Azure DevOps Token pattern to replace the trailing `|\\z` with a negative
lookahead such as `(?![a-zA-Z0-9_~.-])` so boundaries work correctly in
JavaScript.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: be72fa5a-41a6-4863-ac27-490d6a8f9e68
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
eslint.config.jspackage.jsonpackages/backend/package.jsonpackages/frontend/package.jsonpackages/frontend/src/components/results/Results.vuepackages/frontend/src/components/search/Form.vuepackages/frontend/src/components/search/batch/BatchSearchDialog.vuepackages/frontend/src/data/secret-patterns.test.tspackages/frontend/src/data/secret-patterns.tspackages/frontend/src/stores/batchSearchStore.tspackages/frontend/src/stores/grepStore.ts
✅ Files skipped from review due to trivial changes (1)
- packages/backend/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/frontend/src/components/search/batch/BatchSearchDialog.vue
- eslint.config.js
- packages/frontend/package.json
- package.json
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores