Repository navigation
Infisical Sole-Source-of-Truth: Runtime Settings Loader for Tunable Knobs - #1585
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@kody start-review |
This comment has been minimized.
This comment has been minimized.
|
Re-export the intended public settings surface from src/index.ts and have package-level tests and consumers import through that entry point. This prevents the exported schema and utilities from being exercised only through an internal module path. Kody rule violation: Every exported schema or utility ships with tests and must typecheck |
|
Resolve the current session with better-auth and perform the application’s role or tenant authorization against that session before allowing settings to be read, reloaded, or mutated. A standalone verifySessionToken boolean does not satisfy the required current-session authorization contract. Kody rule violation: Authenticate and validate every server mutation |
|
Use two literal ASCII spaces after each period in this operator-facing log message so the required sentence spacing survives rendering. Kody rule violation: Use two spaces between sentences in every human-facing string and agent-written paragraph |
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found critical issues please review the requested changes
- The document exposes the private Infisical project identifier.
- The local-development instructions recommend creating a prohibited dotenv file.
- Unhandled rejection in the SIGHUP refresh handler can crash the process on a failed background refresh.
- The negative-path test hardcodes a client secret.
- The Infisical-mode
getAll()path returns the full secret cache instead of filtering to declared non-secret settings. - This line reads the fallback Infisical automation secret without a server-only module guard.
- appSettings.get() drops the process.env fallback in Infisical mode, so env-only knob values are ignored and emergency switches fail open.
- Strict
=== "true"on USAGE_INGEST_REQUIRE_SCOPED_TOKENS ignores valid true spellings (TRUE/1/yes) and leaves the authz control unenforced.
This comment has been minimized.
This comment has been minimized.
Defensive pin for GHSA-vfj7-8cjw-p6xm (braces stack-exhaustion DoS, pulled in via chokidar and micromatch). Lockfile already resolves the single hoisted copy to 3.0.3, so no lockfile diff; the override guards against future re-resolution to a vulnerable 3.0.x.
a796baa to
7339011
Compare
This comment has been minimized.
This comment has been minimized.
|
Add matching Vitest coverage for each changed source behavior because the PR currently tests the app-settings service changes without covering the other runtime behavior changes. Kody rule violation: Keep type safety and test coverage for source changes |
|
Export the new scheduler utility from src/index.ts and have package-level tests and consumers import it through that entry point because it is currently reachable only through private module aliases. Kody rule violation: Every exported schema or utility ships with tests and must typecheck |
|
Separate the two sentences in the runtime-settings summary bullet with Kody rule violation: Use two spaces between sentences in every human-facing string and agent-written paragraph |
- getWithSource(): same empty-string rule as get() so the admin surface
reports the value actually in effect (Infisical-empty falls to env).
- Record the USAGE_SCHEDULER_ENABLED boot gate: readiness answers with the
boot decision, not the live knob, so a post-boot knob flip cannot wedge
/api/ready into a permanent not_ready/503 restart loop. Doc scoped:
the scheduler gate is boot-applied.
- PUT /api/settings: strict Zod body schema (unknown fields rejected);
rollback failures log loudly instead of being swallowed.
- Regression coverage for the USAGE_INGEST_REQUIRE_SCOPED_TOKENS bool
vocabulary (1/yes/on/TRUE deny, 0/no/off/FALSE allow); replaced the
tautological getBool fallback assertion with a real env-fallback case.
- instrumentation test resets the recorded boot gate between register() runs.
Two findings evaluated as false positives (no code change):
- Test 'hardcoded client secret': values are synthetic fixtures ('id',
'secret', 'bad-secret'); requiring runtime env vars would break hermeticity.
- 'server-only' guard on app-settings.ts: the module is value-imported by the
'use client' FleetQuotaMatrixCard via quota-windows -> provider-manifest;
the guard would fail the production build. Credentials are read only in
init() (nodejs runtime); the client bundle's process.env cannot carry them.
This comment has been minimized.
This comment has been minimized.
- Expose getAppliedSchedulerGate(); GET /api/settings/runtime annotates USAGE_SCHEDULER_ENABLED with appliedValue + restartRequired so the admin surface reports the boot-applied value, not the live knob. - instrumentation test: regression test pinning the boot-gate invariant (post-boot flip leaves the recorded answer false); clear Infisical credential env vars in beforeEach so ambient creds cannot flip the service into Infisical mode mid-suite.
…1585) Per kody re-review: keep the operator-visible Error message (the failure visibility 4180153043 asked for), but replace String(rollbackError) for non-Error throwables with a fixed 'UnknownError' so an arbitrary thrown value is never serialized into logs.
…1585) Reset appSettings and strip ambient Infisical creds in runtime route tests so cloud VMs stay in env-fallback mode. Add scheduler boot-gate unit coverage and annotate GET /api/settings/runtime when the gate is recorded. Co-authored-by: Jay Wedgeworth <jaywedgeworth22@users.noreply.github.com>
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Remove committed Infisical project UUID from app-settings; resolve project id from env only and mark the module server-only. Export strict Zod schemas for settings PUT routes. Inject synthetic Infisical test credentials via vitest setup instead of hardcoding secrets in tests. Align .env.example with the Infisical-backed local dev launcher. Co-authored-by: Jay Wedgeworth <jaywedgeworth22@users.noreply.github.com>
…build The previous commit (af24b1a) added `import "server-only";` to app-settings.ts to address a Kody critical finding (credential-handling module should carry the server-only guard). This re-introduced a defect that an earlier review round (1addfd4) had already identified and deliberately left unfixed: app-settings.ts's read methods are value-imported by src/lib/provider-manifest.ts, which is reachable from the "use client" FleetQuotaMatrixCard (via quota-windows.ts -> AgentsDashboard.tsx). Adding the server-only sentinel anywhere in that import graph — confirmed via `npm run build`, including behind a dynamic import() split (the pattern instrumentation.ts uses to dodge the edge-runtime bundle; it does NOT dodge this specific RSC boundary check) — fails the production build with "'server-only' cannot be imported from a Client Component module". This was breaking build/verify/smoke CI on PR #1585. Revert to the pre-af24b1af state for this one guard; the real security boundary already holds without it — init() (the only call site that reads the Infisical client secret via resolveCredentials()) is invoked exclusively from instrumentation.ts (Node server startup), never from client-reachable code. Documented inline so a future Kody round does not re-trigger this regression. Also drops the now-unused server-only npm dependency and its vitest alias stub, which existed only to support the reverted import. Verified locally: tsc --noEmit, npm run lint, npm test (2850 passed), npm run build (production build succeeds) all green. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@kody please re-review. Pushed commit 6c31b38 which reverts the one build-breaking change from af24b1a (the re-added |
Co-authored-by: Cursor <cursoragent@cursor.com>
3 participants
Summary
appSettingsservice for 17 non-secret runtime knobs, loading Infisical values into an in-memory cache during Node startup and usingprocess.envwhen credentials are unavailable or the initial Infisical load fails.GET,PUT, andPOST /api/settings/runtimeendpoints to inspect effective non-secret values and sources, validate and update supported settings, and force a refresh. Infisical-mode saves persist remotely before updating the cache; env-fallback saves update the local environment.Tests