Open
Conversation
- Add regula-wasi dependency for running OPA policy tests - Add .scripts/run-opa-tests.ts to run tests across all policies - Add npm scripts: test:opa, test:opa:verbose - Add CLAUDE.md with project documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
NikhilVerma
requested review from
ankitpawarcldcvr,
cc-shekher,
pranaySinghDev,
rajatsharma111 and
ssghait007
as code owners
WalkthroughA new OPA test orchestration script was added at Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
🤖 Fix all issues with AI agents
In @.scripts/run-opa-tests.ts:
- Around line 70-77: The current spawnSync call in .scripts/run-opa-tests.ts
assigns proc to the child result but doesn’t surface spawn errors or timeouts;
update the handling after spawnSync (the proc variable) to explicitly check for
proc.error and for proc.status === null (timeout or killed) and set
result.success = false, populate result.output with proc.stdout/proc.stderr as
before and add a result.error or result.failureReason string that includes
proc.error.message or a clear timeout/killed message (include proc.signal if
present) so failures from missing npx or timeouts are surfaced for callers of
this script.
- Around line 91-116: The code currently allows "--filter" with no following
value which yields null/empty and silently continues; update the parsing around
args, filterIndex and filter so that after computing filterIndex you verify
args[filterIndex + 1] exists and is not another flag (e.g., startsWith("-")),
and if it's missing or invalid print a clear error (e.g., console.error) and
exit(1); apply this validation before using filter to filter testDirs (the block
that sets filter and filters testDirs), keeping references to args, filterIndex,
filter and leaving the rest of the flow (findTestDirectories and TERRAFORM_DIR
usage) unchanged.
In `@CLAUDE.md`:
- Around line 7-17: The fenced code block containing the project tree in
CLAUDE.md is missing a language tag (MD040); update the opening fence from ```
to ```text (or another suitable info string) so the block is recognized as a
text code fence; ensure only the opening fence is changed and the rest of the
block content (the tree lines) remains unchanged.
🧹 Nitpick comments (1)
.scripts/run-opa-tests.ts (1)
20-20: Usepath.relativefor cross‑platform paths.
String replace breaks on Windows separators.Proposed fix
-import { join, dirname } from "path"; +import { join, dirname, relative } from "path"; @@ - const relativePath = dir.replace(TERRAFORM_DIR + "/", ""); + const relativePath = relative(TERRAFORM_DIR, dir); @@ - const relativePath = result.path.replace(TERRAFORM_DIR + "/", ""); + const relativePath = relative(TERRAFORM_DIR, result.path);Also applies to: 131-133, 160-163
Comment on lines
+70
to
+77
| const proc = spawnSync("npx", ["regula-wasi", "test", dir], { | ||
| encoding: "utf-8", | ||
| timeout: 120000, | ||
| cwd: process.cwd() | ||
| }); | ||
|
|
||
| result.output = (proc.stdout || "") + (proc.stderr || ""); | ||
| result.success = proc.status === 0; |
There was a problem hiding this comment.
Handle spawnSync errors/timeouts explicitly.
If npx is missing or a timeout occurs, proc.error/proc.status === null should be surfaced.
Proposed fix
const proc = spawnSync("npx", ["regula-wasi", "test", dir], {
encoding: "utf-8",
timeout: 120000,
cwd: process.cwd()
});
- result.output = (proc.stdout || "") + (proc.stderr || "");
- result.success = proc.status === 0;
+ if (proc.error) {
+ result.output = String(proc.error);
+ return result;
+ }
+ if (proc.status === null) {
+ result.output = (proc.stdout || "") + (proc.stderr || "") + "\nProcess terminated (timeout or signal).";
+ return result;
+ }
+ result.output = (proc.stdout || "") + (proc.stderr || "");
+ result.success = proc.status === 0;📝 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
| const proc = spawnSync("npx", ["regula-wasi", "test", dir], { | |
| encoding: "utf-8", | |
| timeout: 120000, | |
| cwd: process.cwd() | |
| }); | |
| result.output = (proc.stdout || "") + (proc.stderr || ""); | |
| result.success = proc.status === 0; | |
| const proc = spawnSync("npx", ["regula-wasi", "test", dir], { | |
| encoding: "utf-8", | |
| timeout: 120000, | |
| cwd: process.cwd() | |
| }); | |
| if (proc.error) { | |
| result.output = String(proc.error); | |
| return result; | |
| } | |
| if (proc.status === null) { | |
| result.output = (proc.stdout || "") + (proc.stderr || "") + "\nProcess terminated (timeout or signal)."; | |
| return result; | |
| } | |
| result.output = (proc.stdout || "") + (proc.stderr || ""); | |
| result.success = proc.status === 0; |
🤖 Prompt for AI Agents
In @.scripts/run-opa-tests.ts around lines 70 - 77, The current spawnSync call
in .scripts/run-opa-tests.ts assigns proc to the child result but doesn’t
surface spawn errors or timeouts; update the handling after spawnSync (the proc
variable) to explicitly check for proc.error and for proc.status === null
(timeout or killed) and set result.success = false, populate result.output with
proc.stdout/proc.stderr as before and add a result.error or result.failureReason
string that includes proc.error.message or a clear timeout/killed message
(include proc.signal if present) so failures from missing npx or timeouts are
surfaced for callers of this script.
Comment on lines
+91
to
+116
| const filterIndex = args.indexOf("--filter"); | ||
| const filter = filterIndex !== -1 ? args[filterIndex + 1] : null; | ||
|
|
||
| console.log("=== Regula OPA Test Runner ===\n"); | ||
|
|
||
| // Check regula-wasi version | ||
| const versionProc = spawnSync("npx", ["regula-wasi", "version"], { | ||
| encoding: "utf-8", | ||
| timeout: 30000 | ||
| }); | ||
|
|
||
| if (versionProc.status !== 0) { | ||
| console.error("Error: regula-wasi is not installed. Run: npm install --save-dev regula-wasi"); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const versionOutput = (versionProc.stdout || "") + (versionProc.stderr || ""); | ||
| const opaVersion = versionOutput.match(/OPA v([\d.]+)/)?.[1] || "unknown"; | ||
| console.log(`Using regula-wasi with OPA v${opaVersion}\n`); | ||
|
|
||
| // Find all test directories | ||
| let testDirs = findTestDirectories(TERRAFORM_DIR); | ||
|
|
||
| if (filter) { | ||
| testDirs = testDirs.filter(d => d.toLowerCase().includes(filter.toLowerCase())); | ||
| console.log(`Filtering to ${testDirs.length} directories matching: ${filter}\n`); |
There was a problem hiding this comment.
Guard against --filter without a value.
Right now it silently becomes falsy; better to fail fast.
Proposed fix
const verbose = args.includes("--verbose");
const filterIndex = args.indexOf("--filter");
- const filter = filterIndex !== -1 ? args[filterIndex + 1] : null;
+ const filter = filterIndex !== -1 ? args[filterIndex + 1] : null;
+ if (filterIndex !== -1 && !filter) {
+ console.error("Error: --filter requires a value.");
+ process.exit(1);
+ }🤖 Prompt for AI Agents
In @.scripts/run-opa-tests.ts around lines 91 - 116, The code currently allows
"--filter" with no following value which yields null/empty and silently
continues; update the parsing around args, filterIndex and filter so that after
computing filterIndex you verify args[filterIndex + 1] exists and is not another
flag (e.g., startsWith("-")), and if it's missing or invalid print a clear error
(e.g., console.error) and exit(1); apply this validation before using filter to
filter testDirs (the block that sets filter and filters testDirs), keeping
references to args, filterIndex, filter and leaving the rest of the flow
(findTestDirectories and TERRAFORM_DIR usage) unchanged.
Comment on lines
+7
to
+17
| ``` | ||
| starchitect-cloudguard/ | ||
| ├── terraform/ # OPA/Rego security policies (IaC scanning) | ||
| │ ├── aws/ # AWS policies (66 service categories) | ||
| │ └── gcp/ # GCP policies (11 service categories) | ||
| ├── runtime/ # TypeScript runtime checks (live cloud API) | ||
| │ ├── aws/ # AWS runtime checks with tests | ||
| │ └── gcp/ # GCP runtime checks with tests | ||
| ├── cli/ # CLI application (oclif-based) | ||
| └── .scripts/ # Build and test scripts | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced block.
MD040 warning; use text (or similar).
Proposed fix
-```
+```text
starchitect-cloudguard/
├── terraform/ # OPA/Rego security policies (IaC scanning)
│ ├── aws/ # AWS policies (66 service categories)
│ └── gcp/ # GCP policies (11 service categories)
├── runtime/ # TypeScript runtime checks (live cloud API)
│ ├── aws/ # AWS runtime checks with tests
│ └── gcp/ # GCP runtime checks with tests
├── cli/ # CLI application (oclif-based)
└── .scripts/ # Build and test scripts</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.20.0)</summary>
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @CLAUDE.md around lines 7 - 17, The fenced code block containing the project
tree in CLAUDE.md is missing a language tag (MD040); update the opening fence
from totext (or another suitable info string) so the block is recognized
as a text code fence; ensure only the opening fence is changed and the rest of
the block content (the tree lines) remains unchanged.
</details>
<!-- fingerprinting:phantom:poseidon:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
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
regula-wasidependency for running OPA/Rego policy tests (uses OPA v1.12.2).scripts/run-opa-tests.tsscript to run tests across all terraform policiestest:opa,test:opa:verboseCLAUDE.mdwith project documentationTest Results
All 73 test directories and 839 IaC configurations pass:
Usage
Test plan
regula-wasiinstalls correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
test:opaandtest:opa:verbose).Documentation