Skip to content

docs: capture implementation plans for upcoming hardening#59

Merged
RawToast merged 3 commits intomasterfrom
plans/review-2026-01-14
Jan 14, 2026
Merged

docs: capture implementation plans for upcoming hardening#59
RawToast merged 3 commits intomasterfrom
plans/review-2026-01-14

Conversation

@RawToast
Copy link
Copy Markdown
Owner

@RawToast RawToast commented Jan 14, 2026

Summary by CodeRabbit

  • New Features

    • OpenAPI 3.1 keyword coverage support
    • CLI safety flags to gate unsafe configs/paths and configurable spec size limit
  • Documentation

    • Added planning guides: maintainability refactor, performance optimizations, and CLI security hardening
  • Tests & Fixtures

    • Added OpenAPI 3.1 fixture, generation snapshot tests, and snapshot checks to lock and verify generated output

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Adds four planning documents (maintainability refactor, OpenAPI 3.1 coverage, performance optimizations, CLI security hardening), a new OpenAPI 3.1 fixture and export, tests and snapshots for keyword coverage and generation stability, and documented implementation steps for multiple refactors and optimizations.

Changes

Cohort / File(s) Change Summary
Plans (new docs)
docs/plans/2026-01-14-maintainability-refactor.md, docs/plans/2026-01-14-performance-optimizations.md, docs/plans/2026-01-14-security-hardening.md, docs/plans/2026-01-14-openapi-31-coverage.md
Added detailed multi-task implementation plans covering schema-generator modularization, shared operation/parameter parsing, OpenAPI 3.1 keyword fixture/test, performance optimizations (Sets, operation lookup, memoization), and CLI safety hardening (flags, size guard, path containment).
OpenAPI 3.1 fixture & exports
packages/specs/resources/openapi-31-keywords.yaml, packages/specs/index.ts
Added OpenAPI 3.1 keywords YAML fixture and exported openapi31KeywordsYamlPath from packages/specs/index.ts.
Tests & snapshots
packages/zenko/src/__tests__/openapi-31-keywords.test.ts, packages/zenko/src/__tests__/__snapshots__/openapi-31-keywords.test.ts.snap, packages/zenko/src/__tests__/generation-output.test.ts*
Added keyword coverage test + snapshot and a generation-output snapshot test (referenced by performance plan) to guard generator output stability.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through plans both big and small,
I split the schema, caught keywords in a thrall,
Snapshots snug to keep output tight,
Sets and lookups racing through the night,
A tiny thump — refactor done just right ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding documentation files that capture implementation plans for security hardening and related maintainability/performance work.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3120edb and 3787e90.

📒 Files selected for processing (2)
  • docs/plans/2026-01-14-maintainability-refactor.md
  • docs/plans/2026-01-14-security-hardening.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T04:45:34.555Z
Learnt from: CR
Repo: RawToast/zenko PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T04:45:34.555Z
Learning: Applies to packages/zenko/src/__tests__/**/*.test.ts : Test specifications live in `packages/zenko/src/__tests__` and use snapshots for output verification

Applied to files:

  • docs/plans/2026-01-14-maintainability-refactor.md
🪛 markdownlint-cli2 (0.18.1)
docs/plans/2026-01-14-security-hardening.md

13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


20-20: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


41-41: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


46-46: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


51-51: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


69-69: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


110-110: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


115-115: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


133-133: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


155-155: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


160-160: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


178-178: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


191-191: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


196-196: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


217-217: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


222-222: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


238-238: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


257-257: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


262-262: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


278-278: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@docs/plans/2026-01-14-openapi-31-coverage.md`:
- Around line 55-67: The OpenAPI fixture declares TupleExample and
ContainsExample but never references them, so their prefixItems/items:false and
contains constraints are never validated; either (A) add references to these
schemas in a path response (e.g., include TupleExample and ContainsExample as
response content schemas under a specific path/method) and update the
corresponding tests to assert those schemas are exported and validated, or (B)
if they are not needed for this test, remove the TupleExample and
ContainsExample declarations from the fixture and update tests to stop expecting
them; locate the TupleExample and ContainsExample definitions to implement one
of these fixes and ensure tests reflect the chosen change.

In `@docs/plans/2026-01-14-security-hardening.md`:
- Line 187: The plan currently validates config-based paths with resolveSafePath
for entry.input, entry.output, and helpersOutput but leaves single-run
positional inputs unchecked, allowing a security bypass; update the input
handling logic so that positional arguments are also passed through
resolveSafePath (relative to the configured output directory) unless a global
flag like --allow-unsafe-paths is set, ensuring the same safety checks apply to
both config-based and positional inputs, and update any functions that parse CLI
inputs (the code that maps positional args to entry.input/entry.output) to call
resolveSafePath before use.
🧹 Nitpick comments (1)
docs/plans/2026-01-14-security-hardening.md (1)

171-184: Clarify intended behavior for absolute paths.

The resolveSafePath logic for absolute paths is ambiguous. When filePath is absolute, it's resolved and then checked against baseDir, but the intent isn't clear:

  • Should absolute paths always be rejected unless allowUnsafe is true?
  • Or should absolute paths under baseDir be allowed?

Consider adding a comment or restructuring for clarity:

 function resolveSafePath(filePath: string, baseDir: string, allowUnsafe: boolean) {
-  const resolved = path.isAbsolute(filePath)
-    ? path.resolve(filePath)
-    : path.resolve(baseDir, filePath)
+  // Resolve relative paths against baseDir, keep absolute paths as-is
+  const resolved = path.isAbsolute(filePath) 
+    ? filePath 
+    : path.resolve(baseDir, filePath)

   if (!allowUnsafe) {
     const relative = path.relative(baseDir, resolved)
+    // Reject paths that escape baseDir (including absolute paths outside baseDir)
     if (relative.startsWith("..") || path.isAbsolute(relative)) {
       throw new Error(`Output path must stay within ${baseDir}`)
     }
   }

   return resolved
 }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b0ba6a and c910609.

📒 Files selected for processing (4)
  • docs/plans/2026-01-14-maintainability-refactor.md
  • docs/plans/2026-01-14-openapi-31-coverage.md
  • docs/plans/2026-01-14-performance-optimizations.md
  • docs/plans/2026-01-14-security-hardening.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-12T04:45:34.555Z
Learnt from: CR
Repo: RawToast/zenko PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T04:45:34.555Z
Learning: Applies to packages/zenko/src/__tests__/**/*.test.ts : Test specifications live in `packages/zenko/src/__tests__` and use snapshots for output verification

Applied to files:

  • docs/plans/2026-01-14-maintainability-refactor.md
  • docs/plans/2026-01-14-security-hardening.md
  • docs/plans/2026-01-14-performance-optimizations.md
  • docs/plans/2026-01-14-openapi-31-coverage.md
📚 Learning: 2026-01-12T04:45:34.555Z
Learnt from: CR
Repo: RawToast/zenko PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T04:45:34.555Z
Learning: This project is a library for generating TypeScript types and path functions from OpenAPI specs with a focus on Zod schemas

Applied to files:

  • docs/plans/2026-01-14-performance-optimizations.md
🪛 markdownlint-cli2 (0.18.1)
docs/plans/2026-01-14-maintainability-refactor.md

13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


20-20: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


41-41: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


46-46: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


51-51: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


69-69: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


93-93: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


98-98: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


116-116: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


138-138: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


143-143: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


161-161: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


174-174: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


179-179: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


200-200: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


205-205: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


221-221: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


237-237: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


242-242: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


258-258: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


304-304: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


309-309: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

docs/plans/2026-01-14-security-hardening.md

13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


20-20: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


38-38: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


58-58: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


63-63: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


79-79: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


98-98: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


103-103: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


119-119: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


128-128: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


147-147: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


152-152: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


168-168: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


189-189: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


194-194: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


221-221: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


226-226: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


241-241: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


249-249: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

docs/plans/2026-01-14-performance-optimizations.md

13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


20-20: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


41-41: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


46-46: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


51-51: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


69-69: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


81-81: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


88-88: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


93-93: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


111-111: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


141-141: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


149-149: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


154-154: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


172-172: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


187-187: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


192-192: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

docs/plans/2026-01-14-openapi-31-coverage.md

13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


20-20: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


69-69: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


77-77: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


93-93: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


115-115: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


120-120: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


135-135: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


140-140: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


145-145: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (1)
docs/plans/2026-01-14-performance-optimizations.md (1)

1-197: LGTM! Well-structured performance optimization plan.

The technical approach is sound:

  • Snapshot testing ensures output stability during refactoring
  • Set-based membership checks are a clear performance win over linear scans
  • Shared operation lookup eliminates redundant iterations
  • Memoization strategy using object identity is appropriate for this use case

The plan properly sequences changes with test verification at each step.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +260 to +299
```ts
function parsePathItemMap(
entries: Record<string, Record<string, unknown>>,
nameMap?: Map<string, string>
): Operation[] {
const operations: Operation[] = []

for (const [path, pathItem] of Object.entries(entries)) {
for (const [method, operation] of Object.entries(pathItem)) {
const normalizedMethod = method.toLowerCase()
if (!isRequestMethod(normalizedMethod)) continue
if (!(operation as { operationId?: string }).operationId) continue

const pathParams = extractPathParams(path)
const requestType = getRequestType(operation, nameMap)
const { successResponse, errors } = getResponseTypes(
operation,
(operation as { operationId: string }).operationId,
nameMap
)
const resolvedParameters = collectParameters(pathItem, operation, spec)
const requestHeaders = getRequestHeaders(resolvedParameters)
const queryParams = getQueryParams(resolvedParameters)

operations.push({
operationId: (operation as { operationId: string }).operationId,
path,
method: normalizedMethod,
pathParams,
queryParams,
requestType,
responseType: successResponse,
requestHeaders,
errors,
})
}
}

return operations
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing spec parameter in parsePathItemMap function.

Line 280 calls collectParameters(pathItem, operation, spec) but spec is not declared in the function scope. This will cause a compilation error when implemented.

🐛 Proposed fix
 function parsePathItemMap(
   entries: Record<string, Record<string, unknown>>,
+  spec: any,
   nameMap?: Map<string, string>
 ): Operation[] {

Also update the usage description at line 302 to reflect the additional parameter:

-Use `parsePathItemMap` for both `spec.paths` and `spec.webhooks` (passing the webhook name as the path key for webhook entries).
+Use `parsePathItemMap(spec.paths, spec, nameMap)` and `parsePathItemMap(spec.webhooks, spec, nameMap)` for paths and webhooks respectively.
📝 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
```ts
function parsePathItemMap(
entries: Record<string, Record<string, unknown>>,
nameMap?: Map<string, string>
): Operation[] {
const operations: Operation[] = []
for (const [path, pathItem] of Object.entries(entries)) {
for (const [method, operation] of Object.entries(pathItem)) {
const normalizedMethod = method.toLowerCase()
if (!isRequestMethod(normalizedMethod)) continue
if (!(operation as { operationId?: string }).operationId) continue
const pathParams = extractPathParams(path)
const requestType = getRequestType(operation, nameMap)
const { successResponse, errors } = getResponseTypes(
operation,
(operation as { operationId: string }).operationId,
nameMap
)
const resolvedParameters = collectParameters(pathItem, operation, spec)
const requestHeaders = getRequestHeaders(resolvedParameters)
const queryParams = getQueryParams(resolvedParameters)
operations.push({
operationId: (operation as { operationId: string }).operationId,
path,
method: normalizedMethod,
pathParams,
queryParams,
requestType,
responseType: successResponse,
requestHeaders,
errors,
})
}
}
return operations
}
function parsePathItemMap(
entries: Record<string, Record<string, unknown>>,
spec: any,
nameMap?: Map<string, string>
): Operation[] {
const operations: Operation[] = []
for (const [path, pathItem] of Object.entries(entries)) {
for (const [method, operation] of Object.entries(pathItem)) {
const normalizedMethod = method.toLowerCase()
if (!isRequestMethod(normalizedMethod)) continue
if (!(operation as { operationId?: string }).operationId) continue
const pathParams = extractPathParams(path)
const requestType = getRequestType(operation, nameMap)
const { successResponse, errors } = getResponseTypes(
operation,
(operation as { operationId: string }).operationId,
nameMap
)
const resolvedParameters = collectParameters(pathItem, operation, spec)
const requestHeaders = getRequestHeaders(resolvedParameters)
const queryParams = getQueryParams(resolvedParameters)
operations.push({
operationId: (operation as { operationId: string }).operationId,
path,
method: normalizedMethod,
pathParams,
queryParams,
requestType,
responseType: successResponse,
requestHeaders,
errors,
})
}
}
return operations
}

Comment on lines +55 to +67
TupleExample:
type: array
prefixItems:
- type: string
- type: integer
items: false
ContainsExample:
type: array
items:
type: string
contains:
type: string
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unused schemas in OpenAPI 3.1 fixture.

TupleExample and ContainsExample schemas are defined but never referenced in any path response or by other schemas. They won't be generated or tested, meaning the prefixItems, items: false, and contains keywords won't be validated.

💡 Suggested fix options

Option 1: Add these schemas to a path response

   /widgets:
     get:
       operationId: listWidgets
       responses:
         "200":
           description: "OK"
           content:
             application/json:
               schema:
                 $ref: "#/components/schemas/WidgetList"
+  /tuples:
+    get:
+      operationId: getTuple
+      responses:
+        "200":
+          description: "OK"
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/TupleExample"
+  /items:
+    get:
+      operationId: getItems
+      responses:
+        "200":
+          description: "OK"
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/ContainsExample"

Also update the test assertions to verify these schemas are exported:

     expect(result).toContain("export const Widget =")
     expect(result).toContain("export const WidgetList =")
+    expect(result).toContain("export const TupleExample =")
+    expect(result).toContain("export const ContainsExample =")

Option 2: Remove unused schemas if not needed for this test

🤖 Prompt for AI Agents
In `@docs/plans/2026-01-14-openapi-31-coverage.md` around lines 55 - 67, The
OpenAPI fixture declares TupleExample and ContainsExample but never references
them, so their prefixItems/items:false and contains constraints are never
validated; either (A) add references to these schemas in a path response (e.g.,
include TupleExample and ContainsExample as response content schemas under a
specific path/method) and update the corresponding tests to assert those schemas
are exported and validated, or (B) if they are not needed for this test, remove
the TupleExample and ContainsExample declarations from the fixture and update
tests to stop expecting them; locate the TupleExample and ContainsExample
definitions to implement one of these fixes and ensure tests reflect the chosen
change.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@docs/plans/2026-01-14-security-hardening.md`:
- Around line 203-216: The resolveSafePath function can be bypassed via symlinks
because it uses path.resolve without following real filesystem links; update
resolveSafePath to resolve symlinks using fs.realpathSync (or
fs.promises.realpath) on the target path (for input files) and on the parent
directory (for output paths) before performing the path.relative(baseDir,
resolved) check, and ensure you check existence (fs.existsSync or stat) for
input files before calling realpath to avoid errors; keep the same error if the
realpath-relative check escapes baseDir and reference the function name
resolveSafePath and variables baseDir/resolved/allowUnsafe in your changes.
🧹 Nitpick comments (1)
docs/plans/2026-01-14-security-hardening.md (1)

13-13: Consider using proper heading levels for task steps.

The static analysis tool flagged multiple instances where bold text is used for step labels instead of proper heading levels (MD036). Additionally, line 13 jumps from h1 to h3, skipping h2 (MD001).

While this doesn't affect functionality, using consistent heading levels would improve document structure and navigation.

Based on static analysis hints.

Also applies to: 20-20, 38-38, 90-90, 95-95, 111-111, 130-130, 135-135, 151-151, 160-160, 179-179, 184-184, 200-200, 226-226, 231-231, 258-258, 263-263, 278-278, 286-286

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c910609 and 3120edb.

📒 Files selected for processing (1)
  • docs/plans/2026-01-14-security-hardening.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/plans/2026-01-14-security-hardening.md

13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


20-20: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


38-38: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


90-90: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


95-95: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


111-111: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


130-130: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


135-135: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


151-151: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


160-160: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


179-179: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


184-184: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


200-200: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


226-226: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


231-231: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


258-258: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


263-263: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


278-278: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


286-286: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (3)
docs/plans/2026-01-14-security-hardening.md (3)

38-88: Good: Positional input validation now included.

The test coverage now validates path safety for both config-based and positional CLI usage, addressing the security bypass concern flagged in previous reviews. The tests correctly verify that both input and output paths require --allow-unsafe-paths when escaping the base directory.


248-256: LGTM: Size guard correctly prevents DoS.

The size check is correctly placed before YAML parsing to prevent resource exhaustion attacks. The 50MB default is reasonable for OpenAPI specifications.


163-176: LGTM: JS config gating implemented correctly.

The approach correctly gates JavaScript config execution behind an explicit opt-in flag while preserving support for JSON and YAML configs. The use of dynamic import with pathToFileURL is appropriate for ES modules.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.35%. Comparing base (7b0ba6a) to head (3787e90).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #59   +/-   ##
=======================================
  Coverage   97.35%   97.35%           
=======================================
  Files          15       15           
  Lines        2194     2194           
=======================================
  Hits         2136     2136           
  Misses         58       58           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RawToast RawToast merged commit cb365fd into master Jan 14, 2026
6 checks passed
@RawToast RawToast deleted the plans/review-2026-01-14 branch January 14, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant