feat: enhance attack vector test suites with consistent naming and categorization #22
Conversation
📝 WalkthroughWalkthroughAdds bounded ReDoS-detection utilities and safe regex testing, restructures and renames many Enclave VM attack test suites to ATK-* taxonomy, introduces ATTACK-MATRIX and SECURITY-AUDIT updates, tightens Redis namespace sanitization, improves Markdown escaping, and pins dependency resolutions in package.json. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/format-perf-summary.mjs (1)
1-1: Fix Prettier formatting issues detected by CI.The pipeline has detected formatting inconsistencies in this file. Run the following command to resolve:
npx prettier --write scripts/format-perf-summary.mjslibs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts (1)
793-805: Fix the escape sequence check in the String.raw test.The test checks for two backslashes when
String.rawproduces only one. Line 799 should verify the single backslash:return result.includes('\n') ? 'raw-works' : 'interpreted';Currently, the check for
'\\n'(two backslashes) will never match the actual result fromString.raw\test\nvalue`, which contains only one backslash followed byn. This causes the test to incorrectly return'interpreted'instead of the expected'raw-works'`.libs/vectoriadb/src/storage/redis.adapter.ts (1)
71-94: Document the 200-character namespace limit in the public interface.The namespace truncation won't affect existing code, but the constraint should be documented in
StorageAdapterConfig(adapter.interface.ts) since users configure namespaces through this public interface. Additionally, consider aligning withFileStorageAdapter, which uses a 100-character limit—both adapters should enforce consistent constraints or the difference should be explicitly documented per adapter.
🤖 Fix all issues with AI Agents
In @libs/enclave-vm/SECURITY-AUDIT.md:
- Line 886: Update the sentence that currently reads "the enclave's tool calling
mechanism" to use the hyphenated compound modifier "tool-calling" so it becomes
"the enclave's tool-calling mechanism"; locate the exact sentence in the
SECURITY-AUDIT.md text block discussing SSRF and modify the phrase to ensure
consistent compound adjective formatting.
- Around line 891-895: The table rows for ATK-SSRF-01 through ATK-SSRF-05 (and
any other SSRF rows containing bare URLs) contain raw URLs; wrap each bare URL
(e.g., http://localhost, http://127.0.0.1, http://[::1], http://0.0.0.0 and
localhost with ports) in backticks so they appear as inline code in the markdown
table cells to satisfy MD034 (e.g., replace http://localhost with
`http://localhost`).
In @libs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.ts:
- Around line 358-379: The test ATK-ASYNC-20 currently only asserts the success
path for generator handling and silently allows a non-success result; update the
test around the Enclave instance (new Enclave(...)) and the enclave.run(...)
call to explicitly assert the outcome by checking result.success (e.g., add
expect(result.success).toBe(true) immediately after awaiting enclave.run) before
asserting result.value, or alternatively add an explicit expectation for the
failure case if generator blocking is acceptable; ensure you still call
enclave.dispose() at the end.
In @libs/enclave-vm/src/__tests__/enclave.io-flood.spec.ts:
- Around line 194-197: The describe block and preceding comment incorrectly
claim the range "ATK-IOFL-08 to ATK-IOFL-12" while only tests 08–11 are present;
update the comment and the describe label string (the describe invocation that
currently reads 'ATK-IOFL-08 to ATK-IOFL-12: Security Level Presets') to the
correct range 'ATK-IOFL-08 to ATK-IOFL-11: Security Level Presets' (or move
ATK-IOFL-12 into this block if that was intended) so the identifier range
matches the actual tests.
- Around line 14-19: The header "Test Categories" block has incorrect ATK-IOFL
ranges; update the three range lines in that comment to reflect the actual test
IDs present: change "ATK-IOFL-08 to ATK-IOFL-12: Security Level Presets" to
"ATK-IOFL-08 to ATK-IOFL-11", change "ATK-IOFL-13 to ATK-IOFL-15: Object
Serialization" to "ATK-IOFL-12 to ATK-IOFL-14", and change "ATK-IOFL-16 to
ATK-IOFL-18: Attack Scenarios" to "ATK-IOFL-15 to ATK-IOFL-17" in the Test
Categories comment so the header aligns with the actual test identifiers in the
file.
In @libs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.ts:
- Around line 86-94: The IPv6 localhost check is using '[::1]' but URL.hostname
yields '::1' (no brackets), so update the conditional in the SSRF check (the
variable hostname in this block) to compare against '::1' (or accept both '::1'
and '[::1]'); modify the clause that currently checks hostname === '[::1]' to
hostname === '::1' (or hostname === '::1' || hostname === '[::1]') so IPv6
loopback hosts are correctly detected.
🧹 Nitpick comments (14)
libs/enclave-vm/SECURITY-AUDIT.md (1)
821-821: Consider replacing "very large" with a more precise descriptor.The phrase "very large strings" is vague. Consider specifying a size threshold (e.g., "strings exceeding 1MB") or using a stronger descriptor (e.g., "massive strings") for clarity.
scripts/format-perf-summary.mjs (1)
19-31: Well-implemented escaping logic with correct ordering.The enhanced
escapeMarkdownfunction properly sanitizes input for safe Markdown table rendering. The critical detail of escaping backslashes first (before other escapes that introduce backslashes) prevents double-escaping issues. The conversion of<and>to HTML entities is particularly important for preventing HTML injection.One optional consideration: Markdown also interprets
*and_as formatting characters. While these are typically rendered literally inside table cells, you might consider escaping them for complete sanitization consistency, especially given the security focus of this PR.Optional: Escape asterisks and underscores for completeness
function escapeMarkdown(text) { if (typeof text !== 'string') return String(text ?? ''); // Escape backslash FIRST (before other escapes that introduce backslashes) // Then escape other markdown special characters for safe table rendering return text .replace(/\\/g, '\\\\') // Backslash must be escaped first .replace(/\|/g, '\\|') // Table cell separator .replace(/`/g, '\\`') // Code spans + .replace(/\*/g, '\\*') // Emphasis/bold + .replace(/_/g, '\\_') // Emphasis/italic .replace(/</g, '<') // HTML tags (use entity to prevent injection) .replace(/>/g, '>') // HTML tags .replace(/\n/g, ' ') // Newlines break table rows .replace(/\r/g, ''); // Carriage returns }libs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.ts (3)
217-239: UnusedblockedUrlsvariable.The
blockedUrlsis destructured but never used in this test. Same applies to tests ATK-SSRF-05 (line 242), ATK-SSRF-11 (line 458), ATK-SSRF-12 (line 496), ATK-SSRF-13 (line 534), ATK-SSRF-14 (line 577), ATK-SSRF-15 (line 615), ATK-SSRF-16 (line 658), ATK-SSRF-17 (line 684), ATK-SSRF-18 (line 709), and ATK-SSRF-19 (line 734).Consider either adding assertions for
blockedUrlsto verify the URL was tracked, or omit the unused variable.
572-651: Consider adding Azure metadata endpoint test.The
blockedPatternsarray includes/metadata\.azure\.com/i(line 64), but there's no explicit test for Azure metadata endpoints. Consider adding a test case for completeness, e.g.,http://169.254.169.254/metadata/instance?api-version=2021-02-01(Azure IMDS) orhttp://metadata.azure.com/.
770-776: Assertion only verifies localhost variants, not all blocked URLs.The assertion at lines 773-774 only verifies that URLs containing "localhost" are blocked. The test also requests
http://something.localwhich should be blocked by the.endsWith('.local')check. Consider asserting that all results are blocked, or add separate assertions for.localdomains.🔎 Proposed enhancement
const results = result.value as { url: string; blocked: boolean }[]; - // At least localhost variants should be blocked - const localhostResult = results.find((r) => r.url.includes('localhost')); - expect(localhostResult?.blocked).toBe(true); + // All localhost and .local variants should be blocked + results.forEach((r) => { + expect(r.blocked).toBe(true); + });libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts (1)
22-32: Test ID gaps between documentation and implementation.The header documents test IDs ATK-FGAD-01 through ATK-FGAD-60, but several IDs are missing from the actual implementation:
- ATK-FGAD-10, ATK-FGAD-30, ATK-FGAD-38, ATK-FGAD-42
- ATK-FGAD-47, ATK-FGAD-48
- ATK-FGAD-52 through ATK-FGAD-54
Consider either:
- Adding the missing test cases
- Adjusting the documented ranges to reflect actual coverage
This doesn't affect security coverage but could cause confusion when cross-referencing tests with documentation.
libs/vectoriadb/src/regex.utils.ts (1)
27-34: Consider consolidating similar patterns.The
nestedQuantifiersandrepeatedGroupspatterns appear nearly identical:
nestedQuantifiers: checks for[*+{]both inside and outside the grouprepeatedGroups: checks for[*+]both inside and outside the groupThe main difference is that
nestedQuantifiersincludes{(for{n,m}quantifiers) whilerepeatedGroupsdoesn't. Consider consolidating these into a single pattern to reduce redundancy, or add a comment explaining why both are needed.Possible consolidation
const REDOS_DETECTION_PATTERNS = { - // Check for nested quantifiers: (a+)+ or (a*)* - nestedQuantifiers: /\([^)]{0,100}[*+{][^)]{0,100}\)[*+{]/, - // Check for alternation with overlapping patterns: (a|ab)* + // Check for nested quantifiers: (a+)+, (a*)*, or (a{1,2})+ + nestedQuantifiers: /\([^)]{0,100}[*+{][^)]{0,100}\)[*+{]/, + // Check for alternation with overlapping patterns: (a|ab)* alternationOverlap: /\([^|]{0,100}\|[^)]{0,100}\)[*+{]/, - // Check for repeated groups with quantifiers: (a+)+ - repeatedGroups: /\([^)]{0,100}[*+][^)]{0,100}\)[*+]/, } as const;libs/enclave-vm/ATTACK-MATRIX.md (2)
332-346: Add language specifier to fenced code block.Based on static analysis hints.
Proposed fix
-``` +```text CWE-94 (Code Injection) → ATK-CINJ-* CWE-400 (Resource Exhaustion) → ATK-RSRC-* ...
349-356: Add language specifier to fenced code block.Based on static analysis hints.
Proposed fix
-``` +```text CVE-2023-29017 → ATK-CVE-2023-29017-* (vm2 exception handler) CVE-2023-30547 → ATK-CVE-2023-30547-* (vm2 AsyncFunction) ...libs/ast-guard/src/pre-scanner/checks/regex-check.ts (1)
408-419: ExtractMAX_SAFE_PATTERN_LENGTHto a shared constant.The constant
MAX_SAFE_PATTERN_LENGTH = 500is defined identically in bothsafePatternTestandsafePatternMatch. Extract it to a module-level constant to avoid duplication and ensure consistency if the value needs to change.🔎 Proposed fix
+/** Maximum pattern length for safe regex operations to prevent ReDoS during analysis */ +const MAX_SAFE_PATTERN_LENGTH = 500; + /** * Safely test a pattern against input with bounded backtracking. * Limits input length and uses linear-time character scanning when possible. * - * @param input - The input string to test (will be truncated to MAX_SAFE_PATTERN_LENGTH) + * @param input - The input string to test (will be truncated if exceeding limit) * @param testPattern - The regex pattern to use for testing * @returns Whether the pattern matches */ function safePatternTest(input: string, testPattern: RegExp): boolean { - // Limit input length to prevent ReDoS - regex patterns shouldn't be longer than this - const MAX_SAFE_PATTERN_LENGTH = 500; const safeInput = input.length > MAX_SAFE_PATTERN_LENGTH ? input.slice(0, MAX_SAFE_PATTERN_LENGTH) : input; try { return testPattern.test(safeInput); } catch { // If regex execution fails, treat as no match return false; } } /** * Safely match a pattern against input with bounded backtracking. * * @param input - The input string to match (will be truncated) * @param matchPattern - The regex pattern to use for matching * @returns The match result or null */ function safePatternMatch(input: string, matchPattern: RegExp): RegExpMatchArray | null { - const MAX_SAFE_PATTERN_LENGTH = 500; const safeInput = input.length > MAX_SAFE_PATTERN_LENGTH ? input.slice(0, MAX_SAFE_PATTERN_LENGTH) : input; try { return safeInput.match(matchPattern); } catch { return null; } }Also applies to: 428-437
libs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.ts (1)
35-46: Consider using try/finally or afterEach for cleanup.Currently, if an assertion fails before
enclave.dispose()is called, the Enclave instance may not be properly cleaned up. Consider using a try/finally block or moving the disposal to an afterEach hook for more robust cleanup.This is a minor concern for test files and the current pattern is common. The impact is limited since tests are isolated.
libs/enclave-vm/src/__tests__/runtime-attack-vectors.spec.ts (3)
153-190: Incomplete ATK-RTME naming migration.The describe block "ATK-RTME-06 to ATK-RTME-08: Array Join Methods" contains three tests, but only the first one (
ATK-RTME-06) has the ATK prefix. The remaining two tests at lines 166 and 178 are missing theATK-RTME-07andATK-RTME-08identifiers.🔎 Suggested naming fixes
- it('should block character array join', async () => { + it('ATK-RTME-07: should block character array join', async () => { ... - it('should block Array.from() + join', async () => { + it('ATK-RTME-08: should block Array.from() + join', async () => {
192-416: Several describe blocks still use legacy numbering.The describe blocks "1.3 String Reverse/Transform Methods", "1.4 Character Code Building", "1.5 Encoding-Based Attacks", and "1.6 proto Building Attacks" use the old numbering scheme instead of the ATK-RTME pattern. Consider updating these to match the header's claimed ATK-RTME-09 through ATK-RTME-20 range for consistency.
For example:
- "1.3 String Reverse/Transform Methods" → "ATK-RTME-09 to ATK-RTME-13: String Reverse/Transform Methods"
- "1.4 Character Code Building" → "ATK-RTME-14 to ATK-RTME-16: Character Code Building"
419-680: Categories 2-8 retain legacy numbering scheme.The remaining test categories (Iterator/Generator Chain Attacks, Error Object Exploitation, Type Coercion Attacks, Known CVE Patterns, Tool Result Attacks, Syntax Obfuscation Attacks, Custom Globals Security) use the old "2.", "3.", etc. numbering instead of the ATK-RTME-21 through ATK-RTME-80 ranges mentioned in the header. Consider completing the naming migration for full consistency with the PR's stated goal of "consistent naming and categorization."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
libs/ast-guard/src/pre-scanner/checks/regex-check.tslibs/enclave-vm/ATTACK-MATRIX.mdlibs/enclave-vm/SECURITY-AUDIT.mdlibs/enclave-vm/src/__tests__/constructor-obfuscation-attacks.spec.tslibs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.tslibs/enclave-vm/src/__tests__/enclave.infinite-loop-attacks.spec.tslibs/enclave-vm/src/__tests__/enclave.io-flood.spec.tslibs/enclave-vm/src/__tests__/enclave.redos-attacks.spec.tslibs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.tslibs/enclave-vm/src/__tests__/function-gadget-attacks.spec.tslibs/enclave-vm/src/__tests__/resource-exhaustion.spec.tslibs/enclave-vm/src/__tests__/runtime-attack-vectors.spec.tslibs/vectoriadb/src/regex.utils.tslibs/vectoriadb/src/storage/redis.adapter.tsscripts/format-perf-summary.mjs
🧰 Additional context used
📓 Path-based instructions (1)
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.tslibs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.tslibs/vectoriadb/src/regex.utils.tslibs/enclave-vm/src/__tests__/enclave.infinite-loop-attacks.spec.tslibs/enclave-vm/ATTACK-MATRIX.mdlibs/enclave-vm/src/__tests__/enclave.redos-attacks.spec.tslibs/vectoriadb/src/storage/redis.adapter.tslibs/enclave-vm/SECURITY-AUDIT.mdlibs/ast-guard/src/pre-scanner/checks/regex-check.tslibs/enclave-vm/src/__tests__/enclave.io-flood.spec.tslibs/enclave-vm/src/__tests__/function-gadget-attacks.spec.tslibs/enclave-vm/src/__tests__/resource-exhaustion.spec.tslibs/enclave-vm/src/__tests__/runtime-attack-vectors.spec.tslibs/enclave-vm/src/__tests__/constructor-obfuscation-attacks.spec.ts
🧬 Code graph analysis (1)
libs/vectoriadb/src/storage/redis.adapter.ts (1)
libs/vectoriadb/src/regex.utils.ts (1)
SAFE_PATTERNS(136-158)
🪛 GitHub Actions: CI
scripts/format-perf-summary.mjs
[warning] 1-1: Prettier formatting issues detected in this file. Run 'npx prettier --write scripts/format-perf-summary.mjs' to fix.
🪛 LanguageTool
libs/enclave-vm/ATTACK-MATRIX.md
[style] ~169-~169: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...BIGINT_EXPONENT | | ATK-RSRC-02 | Block very large BigInt | AST BIGINT_EXPONENT...
(EN_WEAK_ADJECTIVE)
libs/enclave-vm/SECURITY-AUDIT.md
[style] ~821-~821: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... | Block regex on very large strings | NO_REGEX_LITERAL...
(EN_WEAK_ADJECTIVE)
[grammar] ~886-~886: Use a hyphen to join words.
Context: ...nal resources through the enclave's tool calling mechanism. The enclave uses URL ...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
libs/enclave-vm/ATTACK-MATRIX.md
332-332: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
349-349: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
libs/enclave-vm/SECURITY-AUDIT.md
891-891: Bare URL used
(MD034, no-bare-urls)
892-892: Bare URL used
(MD034, no-bare-urls)
895-895: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (31)
libs/enclave-vm/SECURITY-AUDIT.md (3)
523-532: Version history entry for v2.4.0 is well-structured and complete.The new version entry accurately documents the three attack prevention test categories (ATK-REDOS, ATK-ASYNC, ATK-SSRF) with test counts totaling 71 new tests, matching the increase from 1184 to 1255 tests noted in the metadata. The introduction of consistent ATK-* naming aligns with the PR objectives.
800-931: New "Attack Prevention Test Categories" section provides comprehensive test documentation with proper taxonomy.The section clearly documents three major attack vectors with:
- Consistent ATK-* naming scheme (ATK-REDOS, ATK-ASYNC, ATK-SSRF)
- Detailed test matrices with test IDs, descriptions, and defense layers
- Clear distinction between blocked (18, 18, 21) and allowed (5, 6, 3) test cases
- Summary table confirming 71 total tests with 100% pass rate
This aligns well with the PR objective to enhance attack vector test suites with systematic categorization. The structure makes auditing and future maintenance straightforward.
3-6: Metadata updates correctly reflect expanded test suite scope.Version bump to v2.4.0, test suite increase to 1255 tests, and categories count to 28 are internally consistent with the new sections added. The 100% pass rate is maintained, reinforcing the security posture claim.
libs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.ts (4)
1-31: LGTM!The file documentation is thorough with clear categorization following the ATK-SSRF nomenclature. The test categories are well-organized and the defense layers are clearly documented.
266-413: LGTM!Good coverage of file and dangerous protocol blocking, including case-insensitive handling (FILE:// variant) and common attack vectors like gopher, dict, ftp, and ldap protocols.
779-877: LGTM!The double VM operation filtering tests properly verify both allowlist and blocklist patterns, and correctly assert that blocked operations never reach the host tool handler.
879-965: LGTM!Excellent inclusion of positive test cases verifying that legitimate public URLs and allowed operations are not blocked. This provides good balance to the blocking tests and helps prevent over-blocking regressions.
libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts (4)
1-44: Well-organized test documentation with CWE mappings.The header documentation is comprehensive with clear categorization and CWE references. This aligns well with the ATK taxonomy being introduced across the repository.
53-77: LGTM!The helper functions correctly handle multiple valid security outcomes, supporting both AST-level and runtime-level blocking. This defensive testing approach appropriately validates that security measures are effective regardless of which layer catches the attack.
1365-1418: LGTM!The coverage summary test serves as living documentation and validates that the expected attack categories are maintained. The assertion for 10 categories and >30 vectors provides a reasonable coverage baseline.
79-335: Clean test reorganization with proper resource management.The test suite reorganization is well-executed:
- Consistent ATK-FGAD naming convention throughout
- Proper
enclave.dispose()calls after each test- Clear separation between standard and PERMISSIVE mode tests
- Helper functions appropriately handle multiple valid security outcomes
The primitive constructor chain tests comprehensively cover String, Number, Boolean, Array, Object, and RegExp chains.
libs/vectoriadb/src/regex.utils.ts (3)
16-21: LGTM: Well-documented length limit for ReDoS protection.The 500-character limit for detection analysis is a reasonable bound that prevents the detection patterns themselves from being exploited while accommodating most legitimate regex patterns.
36-48: LGTM: Correct implementation of safe pattern testing.The function properly limits input length before applying the pattern and handles errors gracefully. The defensive approach of returning
falseon exceptions prevents test failures from propagating.
57-74: LGTM: Clean refactoring with improved safety.The refactoring to use
safePatternTestmaintains backward compatibility while adding ReDoS protection to the detection patterns themselves. The function behavior remains unchanged from a caller's perspective.libs/enclave-vm/ATTACK-MATRIX.md (1)
1-43: LGTM: Well-structured attack taxonomy documentation.The ATK category reference provides clear mappings between attack prefixes, CWE numbers, and descriptions. This is valuable for understanding the test organization and security coverage.
libs/enclave-vm/src/__tests__/constructor-obfuscation-attacks.spec.ts (2)
1-41: LGTM: Clear test suite organization with ATK-COBS taxonomy.The test suite header provides excellent documentation of the attack categories, CWE mappings, and test range organization. This aligns well with the ATTACK-MATRIX.md documentation.
42-593: LGTM: Consistent test naming following ATK-COBS taxonomy.The test renaming provides clear traceability to the attack matrix documentation while maintaining all existing test logic and assertions.
libs/enclave-vm/src/__tests__/resource-exhaustion.spec.ts (3)
1-41: LGTM: Comprehensive documentation of resource exhaustion test categories.The header provides excellent context on defense layers (AST-level, runtime, VM timeout, worker pool) and test organization. The categorization aligns well with the ATTACK-MATRIX documentation.
42-527: LGTM: Well-organized test categories with consistent naming.The test reorganization into ATK-RSRC categories provides clear structure and traceability. Test logic remains intact while improving discoverability.
556-1609: LGTM: Comprehensive coverage of advanced attack vectors.The extended test suites cover sophisticated attack vectors like memory bombs (Billion Laughs), sort attacks, JSON parser bombs, and template literal injection. The tests appropriately verify that attacks are blocked without causing process crashes.
libs/vectoriadb/src/storage/redis.adapter.ts (3)
5-5: LGTM: Good reuse of centralized safe patterns.Importing
SAFE_PATTERNSfromregex.utilspromotes consistency and leverages the ReDoS-safe patterns defined there.
50-64: LGTM: Clean refactoring to centralize sanitization.Extracting sanitization into a dedicated method improves code organization and reusability. Applying sanitization to both namespace and keyPrefix is appropriate.
76-87: LGTM: Correct ReDoS mitigation pattern with bounded input.The implementation correctly limits input length before applying regex patterns, following the defensive approach introduced in
regex.utils.ts. The sequence of sanitization steps is logical and uses pre-compiled safe patterns.libs/ast-guard/src/pre-scanner/checks/regex-check.ts (2)
439-460: Well-designed bounded detection patterns.The use of
{0,100}instead of unbounded quantifiers like*is an excellent defensive measure to prevent the ReDoS analyzer itself from being vulnerable to the attacks it's trying to detect. The patterns are appropriately structured for their detection purposes.
467-575: LGTM - Safe pattern checking implementation.The
analyzeForReDoSfunction correctly uses the newsafePatternTestandsafePatternMatchwrappers throughout, ensuring the analyzer itself cannot be exploited via malicious input patterns. The scoring logic and thresholds remain unchanged, preserving backward compatibility.libs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.ts (1)
1-26: Well-structured test documentation header.The comprehensive header documents the attack category, defense layers, and test categories with clear ATK-ASYNC-* numbering. This provides excellent traceability for security test coverage.
libs/enclave-vm/src/__tests__/enclave.infinite-loop-attacks.spec.ts (2)
1-36: Excellent CWE reference documentation.The header now includes proper CWE references (CWE-835, CWE-400, CWE-674) which provides excellent traceability for security compliance and audit purposes. The ATK-LOOP category structure with numbered test ranges improves test organization.
41-186: Consistent ATK-LOOP naming applied.The test cases have been consistently renamed from generic descriptions to the ATK-LOOP-XX format. The grouping under descriptive describe blocks (e.g., "ATK-LOOP-01 to ATK-LOOP-07: Obvious Infinite Loop Detection") improves discoverability and traceability.
libs/enclave-vm/src/__tests__/enclave.redos-attacks.spec.ts (2)
1-27: Comprehensive ReDoS test suite with clear defense layer documentation.The test file provides excellent documentation of the multi-layered defense approach (AST-Level NO_REGEX_LITERAL/METHOD, PRESCANNER_REDOS, runtime RegExp blocking). This aligns well with the bounded pattern checking introduced in
regex-check.ts.
329-401: Valuable safe alternatives documentation.The "Safe String Alternatives" section (ATK-REDOS-19 to ATK-REDOS-23) serves dual purposes: verifying safe patterns work correctly and documenting recommended alternatives to regex for developers. This is excellent developer guidance within the test suite.
libs/enclave-vm/src/__tests__/runtime-attack-vectors.spec.ts (1)
1-29: Good ATK-RTME categorization with CWE references.The header updates provide clear ATK-RTME category ranges and relevant CWE references (CWE-94, CWE-693, CWE-200). The detailed security model notes are valuable for understanding the multi-layered protection approach.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI Agents
In @libs/enclave-vm/ATTACK-MATRIX.md:
- Around line 199-217: The ATK-LOOP section in ATTACK-MATRIX.md currently claims
"Tests: ~50" but only lists ATK-LOOP-01 through ATK-LOOP-10; update the header
or the table to be consistent: either change the header "Tests: ~50" to the
actual count (e.g., "Tests: 10") or expand the table to include all test IDs and
descriptions that correspond to the ~50 tests (referencing the tests defined in
enclave.infinite-loop-attacks.spec.ts and the ATK-LOOP-0x identifiers) so the
summary and table match.
- Around line 184-198: The ATK-IOFL section header incorrectly states "Tests:
~15" while the actual spec and test file (enclave.io-flood.spec.ts) include 17
tests (ATK-IOFL-01..ATK-IOFL-17); update the section header to "Tests: 17" and
expand the documentation table to include entries for ATK-IOFL-07 through
ATK-IOFL-17 (or at minimum update the summary count and add placeholder rows
labeled ATK-IOFL-07..ATK-IOFL-17) so the table and header match the test file.
- Around line 160-183: The ATK-RSRC section in ATTACK-MATRIX.md is inconsistent:
the header says "Tests: ~30" but the table lists ATK-RSRC-01 through ATK-RSRC-15
(15 tests); update the section so the count matches the table by either changing
the header to "Tests: 15" (or "~15") or expand the table to include the missing
test rows up to ~30 and ensure corresponding test IDs and descriptions map to
resource-exhaustion.spec.ts entries; reference the "ATK-RSRC: Resource
Exhaustion Prevention (CWE-400)" section and the table rows
(ATK-RSRC-01..ATK-RSRC-15) when making the change.
In @libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts:
- Around line 796-803: The inline comment incorrectly says "(8 chars with
literal backslash)"; update the comment near the `code` template literal in the
test (the String.raw example used to compute `result`) to state the correct
character count "(11 chars with literal backslash)" so it matches the `return
result.length === 11` assertion and the `"test\nvalue"` literal interpretation
used in the `function-gadget-attacks.spec.ts` test.
🧹 Nitpick comments (3)
libs/enclave-vm/SECURITY-AUDIT.md (1)
821-821: Consider replacing "very large strings" with more specific language.Per static analysis feedback, the intensifier "very" could be replaced with a more concrete descriptor (e.g., "Block regex on oversized strings" or "Block regex on strings exceeding N characters"). This is a minor style improvement for clarity in Chill mode.
package.json (1)
65-72: Consider using exact versions instead of caret ranges for security-critical dependencies.The resolutions use caret ranges (e.g.,
^6.14.1), which allow minor and patch updates. For security-critical transitive dependencies—especially in a PR focused on hardening attack vectors and ReDoS protection—exact versions (e.g.,6.14.1without^) provide stronger guarantees against unexpected behavior from automatic updates.🔎 Proposed change to use exact versions
"resolutions": { - "qs": "^6.14.1", - "node-forge": "^1.3.2", - "glob": "^10.5.0", - "jws": "^3.2.3", - "mdast-util-to-hast": "^13.2.1", - "js-yaml": "^4.1.1" + "qs": "6.14.1", + "node-forge": "1.3.2", + "glob": "10.5.0", + "jws": "3.2.3", + "mdast-util-to-hast": "13.2.1", + "js-yaml": "4.1.1" }libs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.ts (1)
56-56: IPv6 pattern in blockedPatterns may not match parsed hostname.The regex
/\[::1\]/(line 56) checks for[::1]with brackets, butURL.hostnamereturns::1without brackets. This pattern will only match the raw URL string before parsing, not after. The post-parse check on line 91 handles this correctly, so this is not a security issue, but the pattern is redundant for parsed hostnames.🔎 Optional: Add bracketless pattern for consistency
/\[::1\]/, + /^::1$/,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
libs/ast-guard/src/pre-scanner/checks/regex-check.tslibs/enclave-vm/ATTACK-MATRIX.mdlibs/enclave-vm/SECURITY-AUDIT.mdlibs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.tslibs/enclave-vm/src/__tests__/enclave.io-flood.spec.tslibs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.tslibs/enclave-vm/src/__tests__/function-gadget-attacks.spec.tspackage.jsonscripts/format-perf-summary.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/format-perf-summary.mjs
🧰 Additional context used
📓 Path-based instructions (1)
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/enclave-vm/SECURITY-AUDIT.mdlibs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.tslibs/enclave-vm/src/__tests__/function-gadget-attacks.spec.tslibs/enclave-vm/src/__tests__/enclave.io-flood.spec.tslibs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.tslibs/enclave-vm/ATTACK-MATRIX.mdlibs/ast-guard/src/pre-scanner/checks/regex-check.ts
🧬 Code graph analysis (1)
libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts (1)
libs/enclave-vm/src/enclave.ts (1)
Enclave(157-739)
🪛 LanguageTool
libs/enclave-vm/SECURITY-AUDIT.md
[style] ~821-~821: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... | Block regex on very large strings | NO_REGEX_LITERAL...
(EN_WEAK_ADJECTIVE)
libs/enclave-vm/ATTACK-MATRIX.md
[style] ~169-~169: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...BIGINT_EXPONENT | | ATK-RSRC-02 | Block very large BigInt | AST BIGINT_EXPONENT...
(EN_WEAK_ADJECTIVE)
🔇 Additional comments (26)
libs/enclave-vm/SECURITY-AUDIT.md (3)
886-886: ✅ Previous style feedback has been incorporated.The hyphenation of "tool-calling mechanism" has been applied correctly per prior review guidance. The markdown is now fully compliant with formatting standards.
891-895: ✅ Markdown URL formatting has been corrected.All bare URLs in the SSRF prevention table have been properly wrapped in backticks (e.g.,
`http://localhost`,`http://127.0.0.1`), satisfying MD034 linting compliance throughout the attack prevention sections.
800-931: Comprehensive new attack prevention test categories are well-structured and documented.The three new subsections (ATK-REDOS, ATK-ASYNC, ATK-SSRF) follow a consistent format with clear test IDs, defense layers, and summary tables. The categorization aligns well with the updated version history and test statistics (1184 → 1255 tests, 25 → 28 categories).
package.json (1)
65-72: No actionable changes needed. All specified dependency versions exist, were published recently (November-December 2025), and have no detected security vulnerabilities. The caret ranges allow for patch-level security updates while maintaining stability.libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts (6)
1-44: Well-structured taxonomy and CWE traceability.The header documentation clearly maps ATK-FGAD test identifiers to CWE references (CWE-94, CWE-693, CWE-1321), which improves traceability for security audits. The category ranges are well-defined.
53-77: LGTM!The helper functions correctly handle the dual-path security model where both AST-level blocking and runtime isolation are valid security outcomes.
22-32: Verify intentional gaps in ATK-FGAD numbering.The header documents ranges (e.g., ATK-FGAD-01 to ATK-FGAD-10), but several IDs within these ranges have no corresponding tests (e.g., ATK-FGAD-10, ATK-FGAD-30, ATK-FGAD-38, ATK-FGAD-42, ATK-FGAD-47-48, ATK-FGAD-52-54). If these gaps are intentional placeholders for future tests, consider adding a note in the documentation. Otherwise, adjust the ranges to match actual coverage.
1368-1420: LGTM!The coverage summary test correctly documents the attack categories and vectors. The
totalVectors > 30assertion is appropriate given the 36 vectors enumerated in theattackCategoriesobject.
97-120: LGTM!Tests consistently follow the pattern of creating an enclave, running code, asserting outcomes with the appropriate helper functions, and disposing the enclave. The use of
expectSecureResultcorrectly handles both AST-level blocking and runtime sandbox isolation as valid security outcomes.
403-426: Good documentation of PERMISSIVE mode behavior.The test clearly documents that
arguments.calleeis available in PERMISSIVE (non-strict) mode and that this is expected behavior, not a vulnerability. This helps maintainers understand the security trade-offs of different modes.libs/enclave-vm/ATTACK-MATRIX.md (1)
3-6: Verify bold claims in header metadata.The header claims "Total Attack Vectors: 500+" and "Test Coverage: 100%". These metrics should be validated or derived from actual test counts. Several sections below show "🔄 Pending Reorganization" status, which may conflict with the 100% coverage claim.
Consider either:
- Automating these metrics from test discovery
- Adding a caveat that coverage refers to implemented (not pending) categories
libs/ast-guard/src/pre-scanner/checks/regex-check.ts (4)
400-423: Well-designed defensive ReDoS detection utilities.The
safePatternTestfunction properly bounds input length and handles exceptions gracefully. This prevents the ReDoS detector itself from being vulnerable to ReDoS attacks.
425-440: LGTM - Consistent implementation with safePatternTest.The
safePatternMatchfunction follows the same defensive pattern assafePatternTest.
442-463: Well-crafted bounded detection patterns.The use of
{0,100}bounded quantifiers instead of unbounded*effectively prevents catastrophic backtracking in the detection patterns themselves. This is a solid defense-in-depth approach.
477-487: LGTM - Safe pattern integration in analyzeForReDoS.The refactored detection logic correctly uses
safePatternTestwith the pre-compiled bounded patterns.libs/enclave-vm/src/__tests__/enclave.async-bomb-attacks.spec.ts (4)
1-28: Well-documented test suite header.The documentation clearly outlines defense layers, test categories with ATK IDs, and package documentation tag. This aligns well with the ATTACK-MATRIX.md structure.
35-46: LGTM - Proper test structure with cleanup.Tests correctly instantiate Enclave, run code, assert on results, and dispose resources. The error message regex pattern appropriately handles implementation variations.
385-406: LGTM - Infinite generator protection test.The test correctly verifies that infinite generators are caught by iteration limits or infinite loop detection.
446-459: LGTM - Positive test case for loops within limits.Good practice to include positive test cases that verify safe patterns work correctly.
libs/enclave-vm/src/__tests__/enclave.io-flood.spec.ts (3)
14-19: Test category ranges now correctly documented.The header test category ranges have been corrected to match actual test IDs:
- ATK-IOFL-01 to ATK-IOFL-04: Console Output Size Limiting (4 tests)
- ATK-IOFL-05 to ATK-IOFL-07: Console Call Count Limiting (3 tests)
- ATK-IOFL-08 to ATK-IOFL-11: Security Level Presets (4 tests)
- ATK-IOFL-12 to ATK-IOFL-14: Object Serialization (3 tests)
- ATK-IOFL-15 to ATK-IOFL-17: Attack Scenarios (3 tests)
194-197: Describe block range corrected.The describe block now correctly states "ATK-IOFL-08 to ATK-IOFL-11" matching the 4 tests in this section.
30-34: LGTM - Well-organized test suite structure.The test suite follows the ATK taxonomy consistently with clear section comments and describe blocks matching the header documentation.
libs/enclave-vm/src/__tests__/enclave.ssrf-prevention.spec.ts (4)
35-116: Well-designed SSRF protection helper with comprehensive blocklist.The
createSSRFProtectedToolHandlerhelper covers an extensive set of SSRF attack vectors including:
- Dangerous protocols (file, gopher, dict, ftp, ldap, etc.)
- Loopback addresses (localhost, 127.0.0.1, ::1, 0.0.0.0)
- Private IP ranges (10.x, 172.16-31.x, 192.168.x, 169.254.x)
- Cloud metadata endpoints (AWS, GCP, Azure)
The IPv6 localhost check on line 91 correctly uses
'::1'(without brackets) sinceURL.hostnamestrips brackets from IPv6 addresses.
118-146: LGTM - Localhost blocking test properly structured.Test correctly verifies the blocking behavior and checks that the URL was recorded in
blockedUrls.
780-877: LGTM - Double VM operation filtering tests.Tests properly verify both allowlist and blocklist patterns for operation name filtering, including verification that blocked operations never reach the host handler.
654-682: The test correctly relies on Node.js URL parsing to normalize decimal IP encoding. Verification confirms thatnew URL('http://2130706433').hostnamereturns127.0.0.1, and the handler properly re-checks the normalized hostname against blocked patterns, catching this obfuscation attempt.
… flood prevention section
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @libs/enclave-vm/ATTACK-MATRIX.md:
- Around line 229-246: The ATK-COBS section header claims "Tests: ~40" but the
table only lists ATK-COBS-01 through ATK-COBS-09; update the metadata in
constructor-obfuscation-attacks.spec.ts and ATTACK-MATRIX.md so the count
matches the table (or expand the table to list ATK-COBS-01..ATK-COBS-40 with
descriptions). Specifically, either change the header "Tests: ~40" to "Tests: 9"
or add entries ATK-COBS-10 through ATK-COBS-40 (with their Description and
Defense Layer columns) to the ATK-COBS table to reflect ~40 tests consistently.
- Around line 247-267: The ATK-FGAD section header in ATTACK-MATRIX.md misstates
the test count as "~50" while function-gadget-attacks.spec.ts documents
ATK-FGAD-01 through ATK-FGAD-60; update the "Tests:" value to match the actual
coverage (e.g., "~60" or "60") and either expand the table to include
representative rows for ATK-FGAD-01..ATK-FGAD-60 or add a note that only a
subset is shown and link to function-gadget-attacks.spec.ts for the full list;
ensure the header "ATK-FGAD: Function Gadget Attacks (CWE-693)" and the file
reference remain unchanged for clarity.
🧹 Nitpick comments (2)
libs/enclave-vm/ATTACK-MATRIX.md (1)
326-336: Defense layer numbering inconsistency.The table lists layers L0 through L5 (6 layers total), but the document mentions "Defense Layer Summary" without explaining this differs from typical L0-L4 (5-layer) models often referenced in security documentation. This is a minor documentation clarity issue—consider adding a brief note that explains the 6-layer model.
libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts (1)
22-32: Test ID gaps in ATK-FGAD numbering sequence.The header documents continuous ranges (e.g., "ATK-FGAD-49 to ATK-FGAD-54: Prototype Pollution Gadgets"), but actual tests have gaps. For example:
- ATK-FGAD-10 is missing between ATK-FGAD-09 and ATK-FGAD-11
- ATK-FGAD-30, ATK-FGAD-38, ATK-FGAD-42, ATK-FGAD-47-48, ATK-FGAD-52-54 are not implemented
Consider either:
- Renumbering tests to be continuous, or
- Updating the header comments to reflect actual implemented ranges
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/enclave-vm/ATTACK-MATRIX.mdlibs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.tslibs/enclave-vm/ATTACK-MATRIX.md
🪛 LanguageTool
libs/enclave-vm/ATTACK-MATRIX.md
[style] ~169-~169: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...BIGINT_EXPONENT | | ATK-RSRC-02 | Block very large BigInt | AST BIGINT_EXPONENT...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Performance
- GitHub Check: Test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
libs/enclave-vm/ATTACK-MATRIX.md (1)
1-15: LGTM! Well-structured security documentation.The attack matrix provides comprehensive categorization with CWE/CVE mappings, defense layer references, and clear test organization. The version, date, and coverage claims are clearly stated.
libs/enclave-vm/src/__tests__/function-gadget-attacks.spec.ts (6)
1-44: LGTM! Well-documented test suite header with clear ATK-FGAD taxonomy.The header provides comprehensive documentation of attack categories, CWE references, and security model explanation. The test category ranges (ATK-FGAD-01 to ATK-FGAD-60) are clearly defined with their purposes.
53-77: LGTM! Well-designed security outcome helpers.The
expectSecureOutcomeandexpectSecureResulthelpers properly handle the dual-security model where both AST-level blocking and runtime sandbox isolation are valid security outcomes. This approach correctly validates that attacks are prevented regardless of which defense layer catches them.
793-808: LGTM! Character count comment is now correct.The comment correctly states "(11 chars with literal backslash)" matching the
result.length === 11assertion. The String.raw test properly validates that escape sequences are preserved as literal characters.
79-79: LGTM! Test suite structure follows ATK-FGAD taxonomy.The top-level describe block correctly references CWE-94 and establishes the test suite namespace for function gadget attack vectors.
1157-1211: Prototype pollution tests correctly verify host isolation.The tests properly verify the critical security property: host
Object.prototype,Array.prototype, andString.prototypeare never polluted regardless of what happens in the sandbox. Theexpect((Object.prototype as Record<string, unknown>)['polluted']).toBeUndefined()assertions confirm host isolation.
1368-1421: LGTM! Coverage summary provides useful documentation.The coverage summary test documents all attack categories and vectors being tested, serving as living documentation. The assertions verify the test suite covers 10 categories with >30 total attack vectors.
… obfuscation and function gadget attacks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @libs/enclave-vm/ATTACK-MATRIX.md:
- Around line 303-317: Update the ATK-CVE table in ATTACK-MATRIX.md to include
the two missing test rows from runtime-attack-vectors.spec.ts: add
ATK-CVE-2023-29017-02 (second exception handler escape test for CVE-2023-29017)
and ATK-CVE-2023-37466-03 (Symbol.for registry access test for CVE-2023-37466),
ensure their Description and Defense Layer match the other CVE entries (e.g.,
"Exception handler escape" / "VM Isolation" and "Symbol.for escape" / "AST
Blocked" respectively), and update the table header/count to reflect the total
tests (~9) and alignment with sections 5.1–5.4 in
runtime-attack-vectors.spec.ts.
🧹 Nitpick comments (3)
libs/enclave-vm/ATTACK-MATRIX.md (3)
169-169: Replace weak adjective "very" with more specific language.The phrase "Block very large BigInt" uses an imprecise intensifier. Consider replacing with a concrete metric or more descriptive term (e.g., "Block excessively large BigInt", "Block BigInt with exponent >10000").
322-340: Clarify intentional partial listing for ATK-UFMT section.The section header declares "Tests: ~35", but the table shows only ATK-UFMT-01 through ATK-UFMT-10 (10 entries). If this is a representative sample (like ATK-FGAD on line 277), add a note to clarify. If the full 35 tests should be documented, expand the table.
🔎 Suggested clarification (if partial listing)
#### ATK-UFMT: Unicode Security (CWE-838) **File:** `unicode-security.spec.ts` **Tests:** ~35 **Status:** 🔄 Pending Reorganization +> **Note:** Only representative tests shown. See `unicode-security.spec.ts` for full ATK-UFMT-01 through ATK-UFMT-35. + | Test ID | Description | Defense Layer |
341-358: Clarify intentional partial listing for ATK-ISOB section.The section header declares "Tests: ~25", but the table shows only ATK-ISOB-01 through ATK-ISOB-09 (9 entries). If this is a representative sample (like ATK-FGAD on line 277), add a note to clarify. If the full 25 tests should be documented, expand the table.
🔎 Suggested clarification (if partial listing)
#### ATK-ISOB: Isolation Breakout (CWE-693) **File:** `isolation-breakout.spec.ts` **Tests:** ~25 **Status:** 🔄 Pending Reorganization +> **Note:** Only representative tests shown. See `isolation-breakout.spec.ts` for full ATK-ISOB-01 through ATK-ISOB-25. + | Test ID | Description | Defense Layer |
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/enclave-vm/ATTACK-MATRIX.md
🧰 Additional context used
📓 Path-based instructions (1)
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/enclave-vm/ATTACK-MATRIX.md
🪛 LanguageTool
libs/enclave-vm/ATTACK-MATRIX.md
[style] ~169-~169: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...BIGINT_EXPONENT | | ATK-RSRC-02 | Block very large BigInt | AST BIGINT_EXPONENT...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performance
- GitHub Check: Test
…criptions for CVE exploits
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/enclave-vm/ATTACK-MATRIX.md (1)
169-169: Consider replacing overused intensifier "very" with more specific language.The phrase "Block very large BigInt" uses an imprecise intensifier. Consider alternatives like "Block unbounded BigInt", "Block massive BigInt exponent", or clarify the threshold in the description.
🔎 Alternative phrasings
Option 1 (Most specific):
-| ATK-RSRC-02 | Block very large BigInt | AST BIGINT_EXPONENT | +| ATK-RSRC-02 | Block unbounded BigInt | AST BIGINT_EXPONENT |Option 2 (Parallel with line 168):
-| ATK-RSRC-02 | Block very large BigInt | AST BIGINT_EXPONENT | +| ATK-RSRC-02 | Block excessive BigInt | AST BIGINT_EXPONENT |
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/enclave-vm/ATTACK-MATRIX.md
🧰 Additional context used
📓 Path-based instructions (1)
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/enclave-vm/ATTACK-MATRIX.md
🪛 LanguageTool
libs/enclave-vm/ATTACK-MATRIX.md
[style] ~169-~169: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...BIGINT_EXPONENT | | ATK-RSRC-02 | Block very large BigInt | AST BIGINT_EXPONENT...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performance
- GitHub Check: Test
🔇 Additional comments (1)
libs/enclave-vm/ATTACK-MATRIX.md (1)
1-432: Excellent comprehensive security attack matrix documentation.This new document successfully consolidates attack vector taxonomy across enclave-vm and ast-guard packages with clear categorization (CWE/CVE-based), test file mappings, defense layer references, and practical test-running commands. All previously flagged test count inconsistencies have been resolved—test headers now match their documentation tables accurately (ATK-RSRC: 15/15 ✓, ATK-IOFL: 17/17 ✓, ATK-LOOP: 10/10 ✓, ATK-COBS: 33/33 ✓, ATK-FGAD: 60 with clarity note ✓, ATK-CVE: 10/10 including previously missing rows ✓). Well-structured with navigable quick references, defense layer summary, and clear metadata.
Summary by CodeRabbit
New Features
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.