fix: route VS Code config suppressions through the Rust config editor#540
fix: route VS Code config suppressions through the Rust config editor#540Darkroom4364 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds a hidden ChangesInternal CLI for scan.ignore_rules + VS Code integration
Sequence Diagram(s)sequenceDiagram
participant User as VS Code User
participant suppressInConfig as suppressInConfig
participant addIgnoreRuleToConfig as addIgnoreRuleToConfig
participant foxguard as foxguard binary
participant config as foxguard::config::add_scan_ignore_rule
User->>suppressInConfig: trigger suppress quick fix
suppressInConfig->>addIgnoreRuleToConfig: rootPath, configPath, relPath, ruleId
addIgnoreRuleToConfig->>foxguard: execFile internal add-scan-ignore-rule
foxguard->>config: scan_root, config path, constructed Finding
config-->>foxguard: (config_path, added)
foxguard-->>addIgnoreRuleToConfig: JSON { config_path, added }
addIgnoreRuleToConfig-->>suppressInConfig: ConfigMutationResult
suppressInConfig->>User: info message (added or already suppressed) with config_path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
vscode-extension/src/extension.ts (1)
216-218: ⚡ Quick winValidate the CLI response shape before resolving success.
JSON.parse(stdout) as ConfigMutationResultdoes not check the integration contract; malformed JSON objects can fall into the duplicate branch becauseresult.addedis just falsy. Validateaddedandconfig_pathbefore returning.Contract validation example
try { - resolve(JSON.parse(stdout) as ConfigMutationResult); + const parsed = JSON.parse(stdout) as Partial<ConfigMutationResult>; + if ( + typeof parsed.config_path !== "string" || + typeof parsed.added !== "boolean" + ) { + reject(new Error("invalid config edit response shape")); + return; + } + resolve(parsed as ConfigMutationResult); } catch (parseError) {🤖 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 `@vscode-extension/src/extension.ts` around lines 216 - 218, The code in the try block parses JSON without validating that the result matches the ConfigMutationResult contract. After the JSON.parse(stdout) call, add validation logic to check that the parsed object contains the required fields added and config_path with the correct types before resolving. If validation fails, reject the promise or throw an error instead of returning potentially malformed data that could cause issues in downstream logic.
🤖 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 `@vscode-extension/src/extension.ts`:
- Around line 182-209: The `execFile` function cannot execute `.cmd` shim files
on Windows, so calling `execFile("npx", ...)` in the binary null check block and
in the `resolveBinary()` function will fail with ENOENT on Windows. Replace the
`execFile` calls with `spawn` using the `{ shell: true }` option, which allows
proper execution of Windows command shims. Additionally, add the `{ cwd:
scanPath }` option to both calls to ensure the workspace-local CLI is resolved
rather than a global version. Apply these changes wherever `npx` or resolved
binaries are executed via `execFile`.
- Around line 205-223: The code in the promise-based execution block does not
prevent race conditions when multiple concurrent mutations target the same
config file. Implement a serialization mechanism that queues or locks mutations
per configPath to ensure that only one execFile call for addIgnoreRuleToConfig
runs at a time for any given config file. Store a map of configPath to pending
promises or queues in the extension, check it before executing execFile, and
update it after the operation completes to serialize concurrent mutations to the
same configuration file.
---
Nitpick comments:
In `@vscode-extension/src/extension.ts`:
- Around line 216-218: The code in the try block parses JSON without validating
that the result matches the ConfigMutationResult contract. After the
JSON.parse(stdout) call, add validation logic to check that the parsed object
contains the required fields added and config_path with the correct types before
resolving. If validation fails, reject the promise or throw an error instead of
returning potentially malformed data that could cause issues in downstream
logic.
🪄 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 Plus
Run ID: 717e95d0-f5f0-40f4-b653-2379dccb41da
📒 Files selected for processing (4)
src/cli.rssrc/main.rstests/integration.rsvscode-extension/src/extension.ts
| if (binary === null) { | ||
| command = "npx"; | ||
| args = [ | ||
| "foxguard", | ||
| "internal", | ||
| "add-scan-ignore-rule", | ||
| "--scan-path", scanPath, | ||
| "--config", configPath, | ||
| "--file", relPath, | ||
| "--rule-id", ruleId, | ||
| ]; | ||
| } else { | ||
| doc.scan.ignore_rules.push({ path: relPath, rules: [ruleId] }); | ||
| } | ||
|
|
||
| fs.writeFileSync(configPath, serializeSimpleYaml(doc)); | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Minimal YAML parser — handles the subset of YAML that .foxguard.yml uses. | ||
| * This avoids adding a js-yaml dependency to the extension. | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function parseSimpleYaml(text: string): any { | ||
| // We use a line-by-line state machine that handles: | ||
| // key: value (string) | ||
| // key: (start mapping) | ||
| // sub: value | ||
| // key: (start sequence) | ||
| // - item (string item) | ||
| // - key: value (mapping item) | ||
| // key2: value2 | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const root: any = {}; | ||
| const lines = text.split("\n"); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const stack: { indent: number; obj: any; key?: string }[] = [ | ||
| { indent: -1, obj: root }, | ||
| ]; | ||
|
|
||
| for (const rawLine of lines) { | ||
| const line = rawLine.replace(/\r$/, ""); | ||
| if (line.trim() === "" || line.trim().startsWith("#")) { | ||
| continue; | ||
| } | ||
|
|
||
| const indent = line.search(/\S/); | ||
| const content = line.trim(); | ||
|
|
||
| // Pop stack to find parent at smaller indent | ||
| while (stack.length > 1 && stack[stack.length - 1].indent >= indent) { | ||
| stack.pop(); | ||
| } | ||
| const parent = stack[stack.length - 1]; | ||
|
|
||
| if (content.startsWith("- ")) { | ||
| // Sequence item | ||
| const itemContent = content.slice(2).trim(); | ||
| // Ensure parent value is an array | ||
| if (parent.key !== undefined && !Array.isArray(parent.obj[parent.key])) { | ||
| parent.obj[parent.key] = []; | ||
| } | ||
| const arr = parent.key !== undefined ? parent.obj[parent.key] : parent.obj; | ||
|
|
||
| if (itemContent.includes(": ")) { | ||
| // Mapping item in a sequence: "- key: value" | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const item: any = {}; | ||
| const colonIdx = itemContent.indexOf(": "); | ||
| const k = itemContent.slice(0, colonIdx).trim(); | ||
| const v = itemContent.slice(colonIdx + 2).trim(); | ||
| item[k] = v; | ||
| arr.push(item); | ||
| stack.push({ indent: indent + 2, obj: item, key: undefined }); | ||
| } else { | ||
| arr.push(itemContent); | ||
| } | ||
| } else if (content.includes(": ")) { | ||
| const colonIdx = content.indexOf(": "); | ||
| const key = content.slice(0, colonIdx).trim(); | ||
| const value = content.slice(colonIdx + 2).trim(); | ||
|
|
||
| if (value === "") { | ||
| // Start of a sub-mapping or sub-sequence (determined later) | ||
| parent.obj[key] = {}; | ||
| stack.push({ indent, obj: parent.obj, key }); | ||
| } else { | ||
| parent.obj[key] = value; | ||
| } | ||
| } else if (content.endsWith(":")) { | ||
| // Key with no value — sub-mapping | ||
| const key = content.slice(0, -1).trim(); | ||
| parent.obj[key] = {}; | ||
| stack.push({ indent, obj: parent.obj, key }); | ||
| } | ||
| command = binary; | ||
| args = [ | ||
| "internal", | ||
| "add-scan-ignore-rule", | ||
| "--scan-path", scanPath, | ||
| "--config", configPath, | ||
| "--file", relPath, | ||
| "--rule-id", ruleId, | ||
| ]; | ||
| } | ||
|
|
||
| return root; | ||
| } | ||
| return await new Promise<ConfigMutationResult>((resolve, reject) => { | ||
| execFile( | ||
| command, | ||
| args, | ||
| { maxBuffer: 1024 * 1024, timeout: 30_000 }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node.js child_process execFile Windows .cmd .bat npx documentation
💡 Result:
In Node.js, child_process.execFile cannot directly execute.cmd or.bat files on Windows because these files are not standalone executables; they require a command interpreter (shell) to run [1][2][3]. Attempting to use execFile on these files typically results in an error (often ENOENT) [4]. Key points regarding Windows, execFile, and tools like npx: 1. Executing.cmd/.bat: To run these files on Windows, you must use child_process.exec or child_process.spawn with the shell option enabled [1][5]. Alternatively, you can spawn cmd.exe explicitly and pass the script as an argument: spawn('cmd.exe', ['/c', 'myScript.cmd']) [1][3]. 2. The npx Issue: On Windows, npm typically installs npx as a shim file named npx.cmd [6]. Because of this, using child_process.execFile('npx',...) fails [6][4]. Developers frequently encounter this issue when using npx-based tools in environments that rely on execFile (such as certain MCP servers or automated task runners) [6][4]. 3. Recommended Workarounds: - Use child_process.exec instead of execFile for these commands, as it spawns a shell by default [1][2]. - If you must use spawn or execFile, pass { shell: true } in the options object [3][4]. - Use absolute paths to the underlying Node.js executable and the intended script (e.g., executing node.exe directly with the script path) to bypass the.cmd shim issue entirely [6]. Official Node.js documentation highlights the distinction: execFile is more efficient on Unix because it avoids spawning a shell, whereas on Windows, the requirement for a shell to process.bat/.cmd files makes exec or shell-enabled spawn necessary [1][3].
Citations:
- 1: https://nodejs.org/api/child%5Fprocess.html
- 2: https://www.freecodecamp.org/news/node-js-child-processes-everything-you-need-to-know-e69498fe970a/
- 3: https://nodejs.org/api/child_process.html
- 4: fix(core): Add shell: true to npm execFile calls for Windows compatibility n8n-io/n8n#24502
- 5: https://nodejs.org/docs/latest-v24.x/api/child_process.html
- 6: Docs: npx-based server configs fail on Windows without cmd /c wrapper modelcontextprotocol/servers#3460
🏁 Script executed:
cd vscode-extension && find . -name "extension.ts" -type f | head -5Repository: 0sec-labs/foxguard
Length of output: 81
🏁 Script executed:
cd vscode-extension && grep -n "resolveBinary\|execFile\|npx" src/extension.ts | head -30Repository: 0sec-labs/foxguard
Length of output: 768
🏁 Script executed:
cd vscode-extension && sed -n '175,225p' src/extension.tsRepository: 0sec-labs/foxguard
Length of output: 1218
🏁 Script executed:
cd vscode-extension && sed -n '583,620p' src/extension.tsRepository: 0sec-labs/foxguard
Length of output: 1059
The npx fallback is broken on Windows; execFile cannot execute .cmd files.
The code calls execFile("npx", ...) at lines 183–206 and in resolveBinary() (line 607), but on Windows npx is a .cmd shim that execFile cannot execute—it will fail with ENOENT regardless of the .cmd extension. The suggested npx.cmd workaround in the diff will not work.
Use one of these approaches instead:
- Replace
execFilewithchild_process.exec(requires careful argument escaping if input is user-controlled). - Use
spawnwith{ shell: true }option. - Resolve the actual npm package path and invoke the Node.js script directly.
Additionally, run npx with { cwd: scanPath } to ensure the workspace-local CLI is resolved, not a global version.
🤖 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 `@vscode-extension/src/extension.ts` around lines 182 - 209, The `execFile`
function cannot execute `.cmd` shim files on Windows, so calling
`execFile("npx", ...)` in the binary null check block and in the
`resolveBinary()` function will fail with ENOENT on Windows. Replace the
`execFile` calls with `spawn` using the `{ shell: true }` option, which allows
proper execution of Windows command shims. Additionally, add the `{ cwd:
scanPath }` option to both calls to ensure the workspace-local CLI is resolved
rather than a global version. Apply these changes wherever `npx` or resolved
binaries are executed via `execFile`.
| return await new Promise<ConfigMutationResult>((resolve, reject) => { | ||
| execFile( | ||
| command, | ||
| args, | ||
| { maxBuffer: 1024 * 1024, timeout: 30_000 }, | ||
| (error, stdout, stderr) => { | ||
| if (error) { | ||
| reject(new Error(stderr.trim() || error.message)); | ||
| return; | ||
| } | ||
|
|
||
| /** | ||
| * Minimal YAML serializer for the config subset we use. | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function serializeSimpleYaml(obj: any, indent: number = 0): string { | ||
| let out = ""; | ||
| const prefix = " ".repeat(indent); | ||
|
|
||
| for (const key of Object.keys(obj)) { | ||
| const value = obj[key]; | ||
| if (Array.isArray(value)) { | ||
| out += `${prefix}${key}:\n`; | ||
| for (const item of value) { | ||
| if (typeof item === "object" && item !== null) { | ||
| const keys = Object.keys(item); | ||
| if (keys.length > 0) { | ||
| out += `${prefix} - ${keys[0]}: ${item[keys[0]]}\n`; | ||
| for (let i = 1; i < keys.length; i++) { | ||
| const v = item[keys[i]]; | ||
| if (Array.isArray(v)) { | ||
| out += `${prefix} ${keys[i]}:\n`; | ||
| for (const subItem of v) { | ||
| out += `${prefix} - ${subItem}\n`; | ||
| } | ||
| } else { | ||
| out += `${prefix} ${keys[i]}: ${v}\n`; | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| out += `${prefix} - ${item}\n`; | ||
| try { | ||
| resolve(JSON.parse(stdout) as ConfigMutationResult); | ||
| } catch (parseError) { | ||
| reject(new Error(`invalid config edit response: ${parseError}`)); | ||
| } | ||
| } | ||
| } else if (typeof value === "object" && value !== null) { | ||
| out += `${prefix}${key}:\n`; | ||
| out += serializeSimpleYaml(value, indent + 2); | ||
| } else { | ||
| out += `${prefix}${key}: ${value}\n`; | ||
| } | ||
| } | ||
| return out; | ||
| }, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect whether config mutations are serialized/locked on the Rust side,
# or whether the extension already queues mutations per config path.
rg -n -C4 'add_scan_ignore_rule|flock|FileExt|NamedTempFile|persist|fs::write|write_all|Mutex|RwLock|configMutation'Repository: 0sec-labs/foxguard
Length of output: 50374
🏁 Script executed:
fd -e rs -e ts | head -20Repository: 0sec-labs/foxguard
Length of output: 451
🏁 Script executed:
rg -l "add_scan_ignore_rule" --type rust --type tsRepository: 0sec-labs/foxguard
Length of output: 126
🏁 Script executed:
rg -A 20 "fn add_scan_ignore_rule" --type rustRepository: 0sec-labs/foxguard
Length of output: 3511
🏁 Script executed:
rg -A 30 "addIgnoreRuleToConfig" vscode-extension/src/extension.tsRepository: 0sec-labs/foxguard
Length of output: 2000
🏁 Script executed:
rg -A 50 "pub fn add_scan_ignore_rule" src/config.rs | head -80Repository: 0sec-labs/foxguard
Length of output: 2072
🏁 Script executed:
rg -B 5 -A 15 "fs::write.*config|write.*yml" src/config.rs | head -60Repository: 0sec-labs/foxguard
Length of output: 2236
🏁 Script executed:
rg "flock|atomic|persist|rename" src/config.rsRepository: 0sec-labs/foxguard
Length of output: 128
🏁 Script executed:
cat -n vscode-extension/src/extension.ts | sed -n '200,240p'Repository: 0sec-labs/foxguard
Length of output: 1503
Verify overlapping suppressions cannot lose config updates — confirmed.
The Rust config editor uses a basic read-modify-write pattern without locking or atomic merge semantics. add_scan_ignore_rule in src/config.rs reads, parses, modifies in memory, serializes, and writes back to disk with no file-level synchronization. Multiple concurrent CLI invocations against the same .foxguard.yml will race: the second write will overwrite the first even if both completed earlier in the pipeline.
Serialize addIgnoreRuleToConfig per configPath in the extension to prevent concurrent mutations to the same config file.
🧰 Tools
🪛 GitHub Check: foxguard
[failure] 206-222: js/no-command-injection (critical)
execFile() called with dynamic argument — risk of command injection
🤖 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 `@vscode-extension/src/extension.ts` around lines 205 - 223, The code in the
promise-based execution block does not prevent race conditions when multiple
concurrent mutations target the same config file. Implement a serialization
mechanism that queues or locks mutations per configPath to ensure that only one
execFile call for addIgnoreRuleToConfig runs at a time for any given config
file. Store a map of configPath to pending promises or queues in the extension,
check it before executing execFile, and update it after the operation completes
to serialize concurrent mutations to the same configuration file.
![foxguard-app[bot]](https://avatars.githubusercontent.com/in/3854821?s=60&v=4){kind=small}
| } | ||
|
|
||
| fn run_internal_add_scan_ignore_rule(args: &InternalAddScanIgnoreRuleArgs) -> i32 { | ||
| let scan_root = Path::new(&args.scan_path); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-path-traversal (CWE-22)
Path::new called with dynamic path — validate input to prevent path traversal
| fn run_internal_add_scan_ignore_rule(args: &InternalAddScanIgnoreRuleArgs) -> i32 { | ||
| let scan_root = Path::new(&args.scan_path); | ||
| let finding_file = { | ||
| let file_path = Path::new(&args.file); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-path-traversal (CWE-22)
Path::new called with dynamic path — validate input to prevent path traversal
|
|
||
| #[test] | ||
| fn test_internal_add_scan_ignore_rule_handles_flow_mapping_config() { | ||
| let repo = TempDir::new().expect("failed to create temp dir"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| let repo = TempDir::new().expect("failed to create temp dir"); | ||
| write_config_file(repo.path(), ".foxguard.yml", "scan: {}\n"); | ||
|
|
||
| let output = foxguard_cmd() |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| ); | ||
|
|
||
| let response: serde_json::Value = | ||
| serde_json::from_slice(&output.stdout).expect("expected JSON response"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| serde_json::from_slice(&output.stdout).expect("expected JSON response"); | ||
| assert_eq!(response["added"], true, "rule should be newly added"); | ||
|
|
||
| let config = fs::read_to_string(repo.path().join(".foxguard.yml")) |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| let config = fs::read_to_string(repo.path().join(".foxguard.yml")) | ||
| .expect("failed to read rewritten config"); | ||
| let value: serde_json::Value = | ||
| serde_yaml_ng::from_str(&config).expect("rewritten config should stay valid YAML"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
|
|
||
| #[test] | ||
| fn test_internal_add_scan_ignore_rule_reports_duplicate_without_rewriting() { | ||
| let repo = TempDir::new().expect("failed to create temp dir"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| "scan:\n ignore_rules:\n - path: src/app.js\n rules:\n - js/no-eval\n", | ||
| ); | ||
|
|
||
| let output = foxguard_cmd() |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| ); | ||
|
|
||
| let response: serde_json::Value = | ||
| serde_json::from_slice(&output.stdout).expect("expected JSON response"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| ); | ||
|
|
||
| let config = | ||
| fs::read_to_string(repo.path().join(".foxguard.yml")).expect("failed to read config"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
|
|
||
| let config = | ||
| fs::read_to_string(repo.path().join(".foxguard.yml")).expect("failed to read config"); | ||
| let ignore_rules = serde_yaml_ng::from_str::<serde_json::Value>(&config) |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
|
|
||
| let config = | ||
| fs::read_to_string(repo.path().join(".foxguard.yml")).expect("failed to read config"); | ||
| let ignore_rules = serde_yaml_ng::from_str::<serde_json::Value>(&config) |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| return root; | ||
| } | ||
| return await new Promise<ConfigMutationResult>((resolve, reject) => { | ||
| execFile( |
There was a problem hiding this comment.
foxguard · CRITICAL · js/no-command-injection (CWE-78)
execFile() called with dynamic argument — risk of command injection
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integration.rs (1)
2969-2998: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen the duplicate test to verify no rewrite occurred.
This currently proves “no duplicate entry,” but not “no rewrite.” A formatting-only rewrite would still pass.
Suggested test hardening
fn test_internal_add_scan_ignore_rule_reports_duplicate_without_rewriting() { let repo = TempDir::new().expect("failed to create temp dir"); write_config_file( repo.path(), ".foxguard.yml", "scan:\n ignore_rules:\n - path: src/app.js\n rules:\n - js/no-eval\n", ); + let original_config = + fs::read_to_string(repo.path().join(".foxguard.yml")).expect("failed to read config"); let output = foxguard_cmd() .current_dir(repo.path()) .args([ "internal", @@ let config = fs::read_to_string(repo.path().join(".foxguard.yml")).expect("failed to read config"); + assert_eq!( + config, original_config, + "duplicate write should not rewrite config content" + ); let ignore_rules = serde_yaml_ng::from_str::<serde_json::Value>(&config) .expect("config should stay valid YAML")["scan"]["ignore_rules"] .as_array() .expect("ignore_rules should be a list") .to_vec();🤖 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 `@tests/integration.rs` around lines 2969 - 2998, The test verifies that a duplicate rule addition doesn't create a new entry in the ignore_rules array, but it doesn't verify that the configuration file wasn't rewritten or reformatted at all. To strengthen the test, capture the content of the .foxguard.yml file (or a hash of it) before executing the duplicate config edit command, then after all assertions pass, verify that the file content remains identical to the original. This ensures the edit command made no changes whatsoever to the file, not just that no new entry was added to the array.
🤖 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.
Nitpick comments:
In `@tests/integration.rs`:
- Around line 2969-2998: The test verifies that a duplicate rule addition
doesn't create a new entry in the ignore_rules array, but it doesn't verify that
the configuration file wasn't rewritten or reformatted at all. To strengthen the
test, capture the content of the .foxguard.yml file (or a hash of it) before
executing the duplicate config edit command, then after all assertions pass,
verify that the file content remains identical to the original. This ensures the
edit command made no changes whatsoever to the file, not just that no new entry
was added to the array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e2a15847-d9cf-4334-8cb4-daf2a712de80
📒 Files selected for processing (2)
src/main.rstests/integration.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.rs
1 participant
Closes #519.
Summary
foxguard internal add-scan-ignore-rulecommand for machine-facing integrationsscan.ignore_rulespaths and report duplicate suppressions cleanlyTesting
cargo test test_internal_add_scan_ignore_rule_cargo run --quiet --bin foxguard -- internal add-scan-ignore-rule ...npm run test(invscode-extension)npx tsc -p ./(invscode-extension)Summary by CodeRabbit
scan.ignore_rulesentries and report whether a rule was newly added via JSON output..foxguard.ymlby invoking the CLI, improving accuracy and surfacing clearer success/error messages.scan.ignore_rules.