feat(provider-security): CPM desired-policy plane for ChefVault - #1
Conversation
Delegate list/switch/status/usage to chefvault so CPM keeps the encrypted API-key vault while ChefGroep OAuth/file profiles stay in one place. Co-authored-by: Cursor <cursoragent@cursor.com>
Lockfile still said 0.4.0 after the release bump in package.json. Co-authored-by: Cursor <cursoragent@cursor.com>
Add fleet/standalone secret backends, opaque chefvault:// policy output, plan/validate/apply/rollback/doctor CLI, and tests that forbid local vault fallback in fleet mode. Co-authored-by: Cursor <cursoragent@cursor.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 40 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 Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesChefVault account driver
Provider security plane
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant PolicyOps
participant PolicySchema
participant ChefVault
participant OpenCodex
CLI->>PolicyOps: Plan or apply provider-security policy
PolicyOps->>PolicySchema: Validate policy references and credentials
PolicySchema-->>PolicyOps: Validation result
PolicyOps->>ChefVault: Inspect opaque credential references
ChefVault-->>PolicyOps: Reference metadata and health status
PolicyOps->>OpenCodex: Write validated policy revision
OpenCodex-->>CLI: Report active revision or diagnostic result
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Remove duplicate policy command registration, fix synchronous validation calls, and document PSP-007 ownership in architecture/CHANGELOG. Co-authored-by: Cursor <cursoragent@cursor.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const planned = await planDesiredPolicy(home); | ||
| const result = await validateDesiredPolicyFile(home, planned.draft); |
There was a problem hiding this comment.
cpm policy validate always generates a new draft with planDesiredPolicy(home) and validates that draft. It never reads JSON supplied on stdin, so an invalid policy can receive a successful validation result for unrelated generated content. Parse and validate stdin when it is supplied, and use draft validation only when no input document is provided.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli.ts
Line: 1092-1093
Comment:
**Validate ignores stdin policy**
`cpm policy validate` always generates a new draft with `planDesiredPolicy(home)` and validates that draft. It never reads JSON supplied on stdin, so an invalid policy can receive a successful validation result for unrelated generated content. Parse and validate stdin when it is supplied, and use draft validation only when no input document is provided.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| if (!options.yes) { | ||
| // no-op; CLI handles confirmation | ||
| } |
There was a problem hiding this comment.
The apply path writes the revision, OpenCodex policy, active pointer, and backup before reaching this options.yes branch, which performs no action. Consequently, cpm policy apply without --yes mutates active configuration without the advertised confirmation safeguard. Require confirmation before any persistent write.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/provider-security/policy-ops.ts
Line: 160-162
Comment:
**Apply bypasses confirmation state**
The apply path writes the revision, OpenCodex policy, active pointer, and backup before reaching this `options.yes` branch, which performs no action. Consequently, `cpm policy apply` without `--yes` mutates active configuration without the advertised confirmation safeguard. Require confirmation before any persistent write.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const scope = providerScope(providerId); | ||
| const keys = await listSecrets(home, scope); | ||
| const active = keys.find((item) => item.active && !item.disabled)?.alias ?? preference?.activeKey ?? "default"; | ||
| const limits = await defaultPoolLimits(providerId); | ||
|
|
||
| const credentialRef = config.secretBackend === "chefvault" | ||
| ? chefVaultRef(`pools/${providerId}`, active) |
There was a problem hiding this comment.
Fleet policy construction turns the local active alias into a chefvault:// reference without verifying that ChefVault contains that reference. A local alias absent from ChefVault therefore produces a syntactically valid policy that applies successfully but leaves OpenCodex unable to resolve its credential. Inspect each generated ChefVault reference and reject planning or apply when the service cannot resolve it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/provider-security/policy-ops.ts
Line: 62-68
Comment:
**Fleet refs use local aliases**
Fleet policy construction turns the local active alias into a `chefvault://` reference without verifying that ChefVault contains that reference. A local alias absent from ChefVault therefore produces a syntactically valid policy that applies successfully but leaves OpenCodex unable to resolve its credential. Inspect each generated ChefVault reference and reject planning or apply when the service cannot resolve it.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const index = revisions.indexOf(selected); | ||
| const previous = index >= 0 ? revisions[index + 1] : revisions[0]; | ||
| if (!previous) throw new Error(`No previous revision before ${selected}`); | ||
|
|
||
| const policy = JSON.parse(await readText(revisionPath(home, previous))) as DesiredPolicyDocument; |
There was a problem hiding this comment.
When a revision is explicitly supplied, this branch finds that revision but then reads revisions[index + 1] instead. For example, rolling back to rev-200 writes and activates rev-100, not the requested archive. Use the selected revision for an explicit rollback; predecessor selection should apply only to an intentional no-argument rollback mode.
Artifacts
Isolated policy rollback reproduction script
- The authored script creates three archived policy revisions in an isolated HOME, runs the real CLI for rev-200, and captures the policy and active pointer, proving the requested revision is skipped.
CLI output after requesting rollback to rev-200
- The executed CLI output and post-command files show rev-100 was returned, written, and activated after explicitly requesting rev-200, confirming the defect.
Policy rollback state before command
- The pre-command capture lists rev-100, rev-200, and rev-300 archives with rev-300 active, establishing the isolated reproduction baseline.
Build output before policy rollback reproduction
- The build log shows npm run build completed successfully before exercising the current CLI, establishing that the reproduction used freshly built code.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/provider-security/policy-ops.ts
Line: 173-177
Comment:
**Named rollback selects predecessor**
When a revision is explicitly supplied, this branch finds that revision but then reads `revisions[index + 1]` instead. For example, rolling back to `rev-200` writes and activates `rev-100`, not the requested archive. Use the selected revision for an explicit rollback; predecessor selection should apply only to an intentional no-argument rollback mode.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| return usageFromPayload(this.id, { | ||
| summary, | ||
| requests, | ||
| totalTokens, | ||
| ocx: root?.ocx, | ||
| usage, | ||
| }); |
There was a problem hiding this comment.
The ChefVault driver constructs an OCX today summary and places it in the payload passed to usageFromPayload, but that helper does not read record.summary. Consumers consequently receive the generic Usage data available message instead of the parsed request and token totals. Preserve a supplied summary in usageFromPayload or return the ChefVault usage result directly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/accounts/index.ts
Line: 277-283
Comment:
**Usage summary is discarded**
The ChefVault driver constructs an `OCX today` summary and places it in the payload passed to `usageFromPayload`, but that helper does not read `record.summary`. Consumers consequently receive the generic `Usage data available` message instead of the parsed request and token totals. Preserve a supplied summary in `usageFromPayload` or return the ChefVault usage result directly.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🤖 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 `@docs/provider-security-plane.md`:
- Around line 63-70: Update the CLI usage block for policy config to include the
existing --standalone option alongside --fleet, preserving the documented
backend and JSON options.
In `@src/accounts/index.ts`:
- Around line 260-283: Update the ChefVault accounts `usage()` method to
construct and return the `UsageResult` directly, preserving its computed
`summary` along with the existing usage fields instead of passing them through
`usageFromPayload`. Add a regression test for `usage()` that verifies the
returned summary includes both request and token counts.
- Around line 253-283: Update usage() to call this.status() and reuse its parsed
status result instead of invoking firstSuccessful and parseJsonOutput again.
Preserve the existing usage extraction, summary construction, and
usageFromPayload behavior using the value returned by status().
In `@src/cli.ts`:
- Around line 1120-1130: Update the doctor command action around
doctorProviderSecurity so it sets process.exitCode to 1 whenever result.ok is
false, while preserving the existing JSON and human-readable output behavior for
all results.
- Around line 1074-1087: Update the human-readable output in the
policy.command("plan") action to remove the unreachable result.warnings branch
and surface the populated result.changes summary instead. Use the existing
PolicyPlanResult changes data returned by planDesiredPolicy, while preserving
the JSON output and existing draft/pool display.
- Around line 1088-1100: Update the policy.command("validate") action to honor
JSON provided through process.stdin, validating that document instead of always
using planDesiredPolicy(home); preserve the planned-draft fallback when no stdin
input is supplied, and keep the existing result output behavior unchanged.
- Around line 1101-1110: Update the policy.command("apply") action to prompt the
user for confirmation before calling applyDesiredPolicy when options.yes is
absent. Abort without applying when the user declines, while preserving the
current unconditional flow for --yes and the existing JSON or success output
after a confirmed application.
- Around line 1131-1146: Update the config command action around the fleetMode
option construction to detect when options.fleet and options.standalone are both
set, then fail loudly with an appropriate error before calling
ensureProviderSecurityConfig. Preserve the existing behavior when either flag is
provided alone or neither is provided.
In `@src/provider-security/chefvault-client.ts`:
- Around line 29-52: Add an AbortSignal.timeout-based deadline to the fetch
calls in health() and inspectRef(), using the existing timeout configuration if
available. Ensure timeout aborts are caught and returned as the same normal
failure result shape as other request errors, so callers do not hang or receive
a rejected promise.
In `@src/provider-security/config.ts`:
- Around line 45-52: Update saveProviderSecurityConfig so it leaves the
caller-provided config unchanged: create a new persisted configuration object
containing the original fields plus schemaVersion and updatedAt, then pass that
object to atomicWrite while preserving the existing validation and path
handling.
- Around line 30-43: Update loadProviderSecurityConfig to handle JSON.parse
failures for the provider security config by returning
defaultProviderSecurityConfig or throwing a typed error that includes the config
path, so callers such as doctorProviderSecurity can report the problem. Also fix
the targetRuntime assignment in loadProviderSecurityConfig so its branches
represent the intended supported runtime behavior instead of returning
"opencodex" unconditionally.
In `@src/provider-security/policy-ops.ts`:
- Around line 43-51: Remove the unnecessary async modifier from
defaultPoolLimits and update its caller around line 65 to stop awaiting the
synchronous result. Delete the empty if block and its comment around lines
160-162, leaving surrounding policy logic unchanged.
- Around line 53-65: Update buildPoolPolicy to accept the already-loaded state
as an argument instead of calling loadState(home) internally. In
planDesiredPolicy, pass its existing state value to every buildPoolPolicy
invocation and remove the per-provider loadState call, preserving the current
provider preference and selection checks.
- Around line 193-207: Update doctorProviderSecurity to declare issues as
PolicyValidationIssue[] and import PolicyValidationIssue in the existing type
import block. Guard the JSON.parse call for the active pointer and the applied
policy file, catching syntax errors and appending descriptive
PolicyValidationIssue entries instead of allowing raw parse exceptions to
escape; preserve the existing successful parsing behavior.
- Line 14: Remove the unused rollbackBackup import from the backup imports in
policy-ops.ts. In every call to the synchronous validateDesiredPolicy function
within the policy operation flows, remove the await and preserve the existing
validation result handling.
- Around line 181-183: Update the rollback flow in applyDesiredPolicy to check
pathExists(targetPath) before calling createBackup, while still performing
atomicWrite to restore the policy when the target file is absent. Preserve the
existing backup behavior when the target file exists.
- Around line 167-179: Update rollbackDesiredPolicy so an explicit revision
argument is treated as the exact rollback target: validate that it exists in
revisions and fail instead of falling back when it does not. When no argument is
provided, derive the rollback target from the active pointer by selecting the
revision immediately older than the active revision, including the existing
no-pointer behavior without applying an extra offset; rename remaining previous
references to target and load/validate that target document.
In `@src/provider-security/policy-schema.ts`:
- Around line 88-102: Update the validation checks in the policy-schema flow so
policy and config agreement is enforced in both directions, not only when
config.fleetMode is true. Reject a fleet policy or chevfvault secret backend
when config is local, while preserving the existing mismatch issues for fleet
configuration and use the declared policy fields as the comparison source.
- Around line 13-19: Broaden RAW_SECRET_PATTERNS by adding a generic
high-entropy API-key pattern that detects credential prefixes and separators
such as sk-, sk_, gsk_, and OpenRouter-style tokens, while preserving the
existing vendor-specific rules. Ensure the pattern also matches the test value
skABCDEFGHIJKLMNOPQRST without relying on the ref-format rule.
In `@src/provider-security/secret-backend.ts`:
- Around line 37-41: Consolidate the fleet-mode backend validation into a single
implementation, using forbidLocalFallback as the canonical function and error
message. Update assertFleetBackend and saveProviderSecurityConfig to reuse that
function instead of duplicating the condition and throw, and re-export it where
needed so existing callers retain access.
In `@src/usage/index.ts`:
- Line 278: Update the target allowlist check in the surrounding usage flow to
derive IDs from accountDrivers using its driver id values instead of the
duplicated literal array. Reuse the existing accountDrivers registration as the
single source of truth while preserving the current target membership behavior.
In `@test/provider-security.test.ts`:
- Around line 48-65: Update the “forbids local vault fallback in fleet mode”
test to mock the ChefVault client instead of making an unmocked network request,
using the existing ChefVault client module symbol. Configure the mock to produce
the intended backend-unavailable scenario, then assert that resolveManagedSecret
rejects specifically with the local vault fallback prohibition message rather
than accepting ChefVault ref unavailable.
- Around line 90-93: Update the assertions in the validateDesiredPolicy test to
keep the existing fleet-opaque-ref-required check and add a separate assertion
requiring an issue whose code starts with "raw-". Remove the OR-based assertion
so raw-credential detection is validated independently.
🪄 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 Plus
Run ID: f0b83a1c-8a45-45ae-8b4a-f5d0d7ddfb19
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
CHANGELOG.mddocs/accounts-and-usage.mddocs/provider-security-plane.mdsrc/accounts/index.tssrc/cli.tssrc/provider-security/chefvault-client.tssrc/provider-security/config.tssrc/provider-security/index.tssrc/provider-security/policy-ops.tssrc/provider-security/policy-schema.tssrc/provider-security/secret-backend.tssrc/provider-security/types.tssrc/types.tssrc/usage/index.tstest/accounts.test.tstest/provider-security.test.ts
| ```bash | ||
| cpm policy plan [--json] | ||
| cpm policy validate [--json] | ||
| cpm policy apply [--yes] [--json] | ||
| cpm policy rollback [revision] | ||
| cpm policy doctor [--json] | ||
| cpm policy config [--fleet] [--backend chefvault|cpm-local] [--json] | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
CLI usage block omits the --standalone flag.
src/cli.ts implements policy.command("config") with a --standalone option ("Disable fleet mode"), but this usage block only documents [--fleet]. Add --standalone here so users know it exists.
📝 Proposed fix
-cpm policy config [--fleet] [--backend chefvault|cpm-local] [--json]
+cpm policy config [--fleet] [--standalone] [--backend chefvault|cpm-local] [--json]📝 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.
| ```bash | |
| cpm policy plan [--json] | |
| cpm policy validate [--json] | |
| cpm policy apply [--yes] [--json] | |
| cpm policy rollback [revision] | |
| cpm policy doctor [--json] | |
| cpm policy config [--fleet] [--backend chefvault|cpm-local] [--json] | |
| ``` |
🤖 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 `@docs/provider-security-plane.md` around lines 63 - 70, Update the CLI usage
block for policy config to include the existing --standalone option alongside
--fleet, preserving the documented backend and JSON options.
| export function forbidLocalFallback(config: ProviderSecurityConfig): void { | ||
| if (config.fleetMode && config.secretBackend !== "chefvault") { | ||
| throw new Error("fleetMode forbids local encrypted vault fallback; set secretBackend=chefvault"); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Consolidate the fleet-backend invariant.
The same rule exists in three places: here, assertFleetBackend in src/provider-security/policy-schema.ts (Lines 129-133), and saveProviderSecurityConfig in src/provider-security/config.ts (Lines 46-48). The three messages differ, so callers get inconsistent error text for one invariant. Keep one implementation and re-export it.
🤖 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 `@src/provider-security/secret-backend.ts` around lines 37 - 41, Consolidate
the fleet-mode backend validation into a single implementation, using
forbidLocalFallback as the canonical function and error message. Update
assertFleetBackend and saveProviderSecurityConfig to reuse that function instead
of duplicating the condition and throw, and re-export it where needed so
existing callers retain access.
There was a problem hiding this comment.
Consolidating the three checks would change user-facing error text that callers and tests rely on, and secret-backend.ts already imports policy-schema.ts, so folding them together risks a circular dependency for no behavioral gain.
There was a problem hiding this comment.
Skipped: comment is from another GitHub bot.
| it("forbids local vault fallback in fleet mode", async () => { | ||
| const home = await tempHome(); | ||
| const config = { | ||
| ...defaultProviderSecurityConfig(), | ||
| fleetMode: true, | ||
| secretBackend: "chefvault" as const, | ||
| }; | ||
| await saveProviderSecurityConfig(home, config); | ||
| await addSecret(home, providerScope("openrouter"), "primary", "skfix3", true); | ||
|
|
||
| expect(() => forbidLocalFallback({ | ||
| ...config, | ||
| secretBackend: "cpm-local", | ||
| })).toThrow(/forbids local encrypted vault fallback/); | ||
|
|
||
| await expect(resolveManagedSecret(home, config, providerScope("openrouter"), "primary", "OPENROUTER_API_KEY")) | ||
| .rejects.toThrow(/local vault fallback is forbidden|ChefVault ref unavailable/); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
Network-dependent assertion weakens the guard verification, and its regex accepts two unrelated failure causes.
At Line 63-64, config.secretBackend is "chefvault", so resolveManagedSecret likely attempts an unmocked ChefVault HTTP call before any local-fallback logic is reached. In a test environment with no ChefVault server listening on the configured URL, the rejection could come purely from a connection failure ("ChefVault ref unavailable"), not from the local-fallback guard itself. The regex /local vault fallback is forbidden|ChefVault ref unavailable/ accepts either outcome, so this assertion would still pass even if the "forbid local fallback" guard were broken or removed, as long as the network call fails first. This also makes the test's outcome depend on the environment having no service on the ChefVault URL, which is a fragile assumption for CI.
Mock the ChefVault client (e.g., with vi.mock on chefvault-client.js) to deterministically simulate an unreachable or reachable backend, and assert specifically on the local-fallback rejection message.
🤖 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 `@test/provider-security.test.ts` around lines 48 - 65, Update the “forbids
local vault fallback in fleet mode” test to mock the ChefVault client instead of
making an unmocked network request, using the existing ChefVault client module
symbol. Configure the mock to produce the intended backend-unavailable scenario,
then assert that resolveManagedSecret rejects specifically with the local vault
fallback prohibition message rather than accepting ChefVault ref unavailable.
| const result = validateDesiredPolicy(badPolicy, config); | ||
| expect(result.ok).toBe(false); | ||
| expect(result.issues.some((item) => item.code === "fleet-opaque-ref-required")).toBe(true); | ||
| expect(result.issues.some((item) => item.code.startsWith("raw-") || item.code === "fleet-opaque-ref-required")).toBe(true); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Redundant assertion doesn't add coverage beyond the previous line.
Line 92 already proves result.issues contains an item with code === "fleet-opaque-ref-required". The OR condition on Line 93 (item.code.startsWith("raw-") || item.code === "fleet-opaque-ref-required") is trivially satisfied by the same fact already established, so it doesn't verify that raw-credential detection codes (raw-*) are actually present. If raw-credential detection regresses, this test still passes.
Split this into a dedicated assertion that checks specifically for a raw-* code, independent of fleet-opaque-ref-required.
🐛 Proposed fix
expect(result.issues.some((item) => item.code === "fleet-opaque-ref-required")).toBe(true);
- expect(result.issues.some((item) => item.code.startsWith("raw-") || item.code === "fleet-opaque-ref-required")).toBe(true);
+ expect(result.issues.some((item) => item.code.startsWith("raw-"))).toBe(true);📝 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.
| const result = validateDesiredPolicy(badPolicy, config); | |
| expect(result.ok).toBe(false); | |
| expect(result.issues.some((item) => item.code === "fleet-opaque-ref-required")).toBe(true); | |
| expect(result.issues.some((item) => item.code.startsWith("raw-") || item.code === "fleet-opaque-ref-required")).toBe(true); | |
| const result = validateDesiredPolicy(badPolicy, config); | |
| expect(result.ok).toBe(false); | |
| expect(result.issues.some((item) => item.code === "fleet-opaque-ref-required")).toBe(true); | |
| expect(result.issues.some((item) => item.code.startsWith("raw-"))).toBe(true); |
🤖 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 `@test/provider-security.test.ts` around lines 90 - 93, Update the assertions
in the validateDesiredPolicy test to keep the existing fleet-opaque-ref-required
check and add a separate assertion requiring an issue whose code starts with
"raw-". Remove the OR-based assertion so raw-credential detection is validated
independently.
…ng and named rollback Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
… policy validation Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
1 participant
Summary
chefvault://policy outputcpm policy plan|validate|apply|rollback|doctor|configDepends on ChefVault provider-security HTTP (
OnlineChefGroep/chefgroep-vault#86).Test plan
npm test -- test/provider-security.test.tstsc --noEmitMade with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Greptile Summary
This change adds desired-policy management, ChefVault-backed credential support, OpenCodex policy output, revision archives, and ChefVault account usage integration.
The policy workflows need correction before merge: policy validation can approve a generated draft instead of the policy provided on stdin; apply mutates policy state without requiring
--yes; fleet plans can publish references that ChefVault cannot resolve; and a named rollback restores the preceding archive rather than the requested revision. ChefVault usage output also drops the parsed daily request and token summary.T-Rex validation blocked
The Greptile artifact-upload capability/API tool was unavailable for the stdin-validation, unconfirmed-apply, ChefVault-reference-resolution, and ChefVault-usage checks. Their executed reproduction evidence could not be published as uploaded artifacts.
Confidence Score: 4/5
What T-Rex did
Comments Outside Diff (2)
General comment
cpm policy apply --jsonwithout--yesexited successfully and wrote the OpenCodex policy, active pointer, revision archive, and backup archive.src/provider-security/policy-ops.ts:143-158persists state before the no-opoptions.yesbranch at lines 160-162;src/cli.ts:1101-1109calls apply without prompting.applyDesiredPolicy, or reject unconfirmed calls before any write.General comment
rev-100,rev-200, and activerev-300, runningcpm policy rollback rev-200 --jsonreturnsrev-100, writes therev-100policy (marker=archive-predecessor) to OpenCodex, and setsactive.jsontorev-100.src/provider-security/policy-ops.ts:173-177, the explicit argument becomesselected, but the code calculatesprevious = revisions[index + 1]and readspreviousrather thanselected. The same predecessor is then persisted as active at lines 184-190.revisionis supplied, load, validate, write, and activate that exact revision. Retain predecessor selection only for the no-argument rollback behavior, if that is intended; add a regression test asserting explicitrollback rev-200writes and activatesrev-200.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(provider-security): CPM desired-pol..." | Re-trigger Greptile