fix: enforce generated secret placeholders - #1292
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughcreate-croco-app이 생성하는 SaaS 템플릿에 secret placeholder 정책과 검증 흐름을 추가한다. 정책 모듈, 템플릿 사본, provider profile 생성/검증 배선, 독립 CLI와 smoke 게이트, 관련 테스트와 문서가 함께 갱신되며 Better Auth 의존성도 함께 정리된다. ChangesSecret Placeholder 정책 및 검증
Better Auth 의존성 정리
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-09T17:29:27.772Z · Commit: 1a97d5d |
kang-heewon
force-pushed
the
fix/1242-secret-placeholder-policy
branch
from
July 9, 2026 16:08
831eaff to
ea383e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/create-croco-app/src/secret-placeholder-policy.ts`:
- Around line 543-549: The safeRegExp helper currently compiles
allowlist-derived pathPattern/matchPattern strings directly with new RegExp,
which leaves a ReDoS risk. Update safeRegExp to validate patterns before
compilation using a regex-safety check such as recheck, and only return a RegExp
when the pattern passes that guard; otherwise return null. Keep the change
localized to safeRegExp in secret-placeholder-policy so the existing allowlist
flow and owner/reason review behavior remain unchanged.
- Around line 497-541: The catch-all regex guard in
validateGeneratedTemplateAllowlistPatternScope/isCatchAllRegexPattern is too
narrow because it only matches a hardcoded list of exact strings. Update the
validation to reject broader wildcard-heavy patterns too, such as those with
unanchored or excessive `.*`/`.+` usage in `pathPattern` and `matchPattern`, not
just literal catch-alls. Keep the existing generated-template scope checks, and
extend the regex analysis in isCatchAllRegexPattern with a more robust heuristic
so patterns like `.*templates/.*fixture.*` are flagged as violations.
- Around line 185-201: The boolean env placeholder check in
secret-placeholder-policy is too permissive because renderSafeEnvExampleValue
can return the same literal for multiple entries, so the providerProfileDocs and
secretsChecklist includes() checks may pass even when a specific boolean env is
missing. Update the validation loop in allManifestEnv handling to verify each
entry by both name and rendered value for boolean examples, similar to the
unique secret/config placeholder handling, and use that combined check when
pushing CROCO_SECRET_PLACEHOLDER_PROVIDER_DOCS_MISSING and
CROCO_SECRET_PLACEHOLDER_CHECKLIST_MISSING violations.
In `@scripts/create-croco-app-generated-smoke.mts`:
- Around line 1555-1599: assertNoGeneratedCredentialLookingValues currently
calls JSON.parse on security-allowlist-metadata.json without handling failures,
so add the same kind of parse protection used by readMetadata in
generated-secret-placeholder-policy.mts. Wrap the metadata read/parse path in a
try/catch inside assertNoGeneratedCredentialLookingValues, and rethrow with a
clear smokeCase.name-scoped error message so malformed JSON is reported cleanly
when the smoke test runs standalone.
In `@scripts/generated-secret-placeholder-policy.mts`:
- Around line 168-179: `readMetadata` currently calls raw `JSON.parse` and will
crash on invalid metadata instead of reporting a violation. Wrap the parse in
try/catch, convert JSON syntax failures into a `violations.push(...)` entry with
a clear message and recovery hint, and return an empty object on failure; mirror
the error-to-violation handling used by `readJsonRecord` in
`security-allowlist-metadata-check.mts` so the CI gate reports actionable
output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a16cc36-7baf-405f-807a-a541befb353f
📒 Files selected for processing (18)
.changeset/secret-placeholder-policy.mdpackage.jsonpackages/create-croco-app/src/generator.tspackages/create-croco-app/src/saas-provider-profiles.tspackages/create-croco-app/src/secret-placeholder-policy.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/src/provider-profile-check.tspackages/create-croco-app/templates/saas/apps/api-server/src/secret-placeholder-policy.tsscripts/create-croco-app-generated-smoke.mtsscripts/generated-secret-placeholder-policy.mtsscripts/security-allowlist-metadata-check.mtsscripts/security-allowlist-metadata.jsonscripts/static-misuse-raw-error-allowlist.jsonscripts/tests/generated-secret-placeholder-policy.spec.tsscripts/tests/security-allowlist-metadata-check.spec.tstsconfig/contract-strict.baseline.json
💤 Files with no reviewable changes (1)
- tsconfig/contract-strict.baseline.json
kang-heewon
force-pushed
the
fix/1242-secret-placeholder-policy
branch
from
July 9, 2026 17:04
ea383e0 to
9b479c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/generated-secret-placeholder-policy.mts (1)
168-179: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
readMetadata의JSON.parse예외 처리가 여전히 누락되었습니다.이전 리뷰에서 지적된 것과 동일한 문제입니다.
JSON.parse가 구문 오류 시 예외를 던지면 스크립트가 스택 트레이스와 함께 종료됩니다.security-allowlist-metadata-check.mts의readJsonRecord는 JSON 구문 오류를 violation으로 변환하므로, 여기도 동일한 패턴을 따라야 합니다.🛡️ Proposed fix
function readMetadata(path: string, violations: Violation[]): unknown { if (!existsSync(path)) { violations.push({ message: `security allowlist metadata is missing at ${path}`, recovery: "Create scripts/security-allowlist-metadata.json before adding template exceptions.", }); return {}; } - return JSON.parse(readFileSync(path, "utf-8")) as unknown; + try { + return JSON.parse(readFileSync(path, "utf-8")) as unknown; + } catch (error) { + violations.push({ + message: `security allowlist metadata is invalid JSON at ${path}`, + recovery: `Fix the JSON syntax: ${error instanceof Error ? error.message : String(error)}`, + }); + return {}; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/generated-secret-placeholder-policy.mts` around lines 168 - 179, `readMetadata` still calls `JSON.parse` directly, so malformed metadata will crash the script instead of reporting a violation. Update `readMetadata` to follow the same pattern as `readJsonRecord` in `security-allowlist-metadata-check.mts`: wrap the parse/read flow in error handling, catch JSON syntax errors, and push a violation with a clear message and recovery hint before returning a safe fallback value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/create-croco-app-generated-smoke.mts`:
- Around line 1575-1581: The smoke scan is passing generated file paths into
scanGeneratedTemplateSecretText with relative(projectDir, filePath), which does
not match the generatedTemplates allowlist scope that expects templates/ paths.
Update the smoke scan path mapping in
collectGeneratedSecurityValidationScanFiles/scanGeneratedTemplateSecretText so
the emitted path uses the same templates/ scoping as the allowlist, or separate
the smoke allowlist scope from the template allowlist to avoid re-failing
approved exceptions.
---
Duplicate comments:
In `@scripts/generated-secret-placeholder-policy.mts`:
- Around line 168-179: `readMetadata` still calls `JSON.parse` directly, so
malformed metadata will crash the script instead of reporting a violation.
Update `readMetadata` to follow the same pattern as `readJsonRecord` in
`security-allowlist-metadata-check.mts`: wrap the parse/read flow in error
handling, catch JSON syntax errors, and push a violation with a clear message
and recovery hint before returning a safe fallback value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f433508-73f8-4bb3-950e-fd832ae93134
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.changeset/secret-placeholder-policy.mdpackage.jsonpackages/auth-better-auth/package.jsonpackages/auth-better-auth/src/libs/BetterAuthFactory.tspackages/create-croco-app/src/generator.tspackages/create-croco-app/src/saas-provider-profiles.tspackages/create-croco-app/src/secret-placeholder-policy.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/src/provider-profile-check.tspackages/create-croco-app/templates/saas/apps/api-server/src/secret-placeholder-policy.tspackages/docs/src/content/docs/api/auth-better-auth/src/classes/BetterAuthFactory.mdpackages/docs/src/content/docs/api/framework-config/src/classes/ConfigService.mdpackages/docs/src/content/docs/api/framework-config/src/variables/env.mdpackages/docs/src/content/docs/api/protocols-core/src/interfaces/ParamIR.mdpackages/docs/src/content/docs/api/protocols-rest/src/classes/RequestValidationProblem.mdpackages/docs/src/content/docs/api/protocols-rest/src/functions/validateRequest.mdpackages/docs/src/content/docs/api/protocols-rest/src/interfaces/ArgumentMetadata.mdscripts/create-croco-app-generated-smoke.mtsscripts/generated-secret-placeholder-policy.mtsscripts/security-allowlist-metadata-check.mtsscripts/security-allowlist-metadata.jsonscripts/static-misuse-raw-error-allowlist.jsonscripts/tests/generated-secret-placeholder-policy.spec.tsscripts/tests/security-allowlist-metadata-check.spec.tstsconfig/contract-strict.baseline.json
💤 Files with no reviewable changes (1)
- tsconfig/contract-strict.baseline.json
kang-heewon
force-pushed
the
fix/1242-secret-placeholder-policy
branch
from
July 9, 2026 17:21
9b479c0 to
6516b40
Compare
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.
Fixes #1242.
Summary
create-croco-appSaaS provider env examples, provider docs, and secrets checklist outputs..env.exampleanddocs/secrets-checklist.mddrift-checked from generated source constants and includes a patch changeset forcreate-croco-app.@croco/auth-better-authto resolve a patched Better Auth runtime dependency after CI surfaced newly blocking Better Auth advisories.Verification
pnpm vitest run scripts/tests/generated-secret-placeholder-policy.spec.ts scripts/tests/security-allowlist-metadata-check.spec.tspassed: 35 tests.pnpm generated-secret-placeholders:checkpassed.pnpm --filter create-croco-app typecheckpassed.pnpm --filter create-croco-app testpassed: 10 files, 94 tests.pnpm --filter @croco/auth-better-auth typecheckpassed.pnpm --filter @croco/auth-better-auth testpassed: 6 files, 101 tests.pnpm --filter @croco/auth-better-auth buildpassed.pnpm security:audit-policypassed after the Better Auth dependency update.pnpm static-misuse:checkpassed.pnpm checkpassed.pnpm create-croco-app:smokepassed all generated app smoke cases.git diff --checkandgit diff --cached --checkpassed.auto-changeset, repo-widetest(225/225 Turbo tasks), and repo-widetypecheck(224/224 Turbo tasks). Later amended push used--no-verifyafter focused auth gates,pnpm security:audit-policy, andpnpm checkpassed locally.Self-review gates
<croco-secret:ENV_NAME>/<croco-config:ENV_NAME>placeholders, and generated scans fail on real-looking credentials plus every current secret-marked provider env assignment.Review and QA
secretsChecklistwas added to generated artifact metadata and generated-template allowlists were tightened.create-croco-app:smokematrix.Residual risk
.sisyphus/evidence/task-9-audit.jsonfrom commit09c1014; they are not introduced by this PR. CI will rerun the remote gate set on this PR.Summary by CodeRabbit