feat(config): add agentsync config command and [security] schema section - #185
Conversation
…ction Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011L7s33UCjpQkreXW7amAw2
|
Warning Review limit reached
More reviews will be available in 46 minutes and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a Changesagentsync config command and security schema
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/commands/__tests__/config.test.ts`:
- Around line 65-72: The restore() function that cleans up environment variables
and temporary directories is currently called manually at the end of each test,
which means cleanup is skipped if any assertion fails before that line. Wrap the
restore() function call in an afterEach hook at the test suite level to
guarantee cleanup runs unconditionally after every test, then remove all
individual await restore() calls from the end of each test function (such as
those at the end of the test cases in the file).
In `@src/commands/config.ts`:
- Around line 171-179: The performConfigSet function writes and pushes
user-supplied configuration values without sanitizing them for secrets, which
could expose literal secrets in git history. Before calling writeConfig with
validated.data, import and apply the sanitizer from src/core/sanitizer.ts to
check for hard never-sync patterns in the validated.data object. If the
sanitizer detects secrets or blocked patterns, abort the operation and throw an
error with clear guidance to the user explaining what sensitive content was
detected and how to fix it. Only proceed with writeConfig, git.addAll,
git.commit, and git.push if the sanitization passes successfully.
🪄 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: bdf3daca-0282-499c-91c1-7ca9313c9daf
📒 Files selected for processing (10)
README.mddocs/commands.mdsrc/cli.tssrc/commands/__tests__/config.test.tssrc/commands/__tests__/integration.test.tssrc/commands/config.tssrc/commands/init.tssrc/config/__tests__/schema.test.tssrc/config/schema.tssrc/test-helpers/fixtures.ts
…g values for secrets - inline literal guards before each setByPath assignment (CodeQL barrier) - refuse literal credentials in plaintext config values (allowlist exempt) - afterEach teardown in config tests for guaranteed cleanup Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011L7s33UCjpQkreXW7amAw2
Why
There was no way to change vault configuration without hand-editing
agentsync.tomlinside the vault clone — enabling an agent, tuning the daemon debounce, or (soon) setting secret-handling policy all meant editing TOML by hand and remembering to commit + push it. This PR adds a proper config surface and lays the schema foundation the next PR (secret-scanner) builds on.Second PR in the key-lifecycle / discovery / daemon / configurability series (follows #184).
What
agentsync config list | get <key> | set <key> <value>— view or change config by dotted key (agents.vscode,sync.debounceMs,security.secretScan, …).setvalidates the mutated config against the full Zod schema (so an out-of-range debounce or a bad enum is rejected before anything is written), then — becauseagentsync.tomlis shared across machines — reconciles fast-forward, commits, and pushes, exactly likekey add.[security]schema section —secretScan(standard/strict/off),allowSecretValues(string[]),redactBase64Values(bool), all optional with safe defaults. The schema is non-.strict(), so oldagentsync.tomlfiles load unchanged and no vault version bump is needed. The section is the config surface only — the push-time secret scanner starts honouring it in the follow-up PR (marked inert in the schema comment and docs so nobody is misled).version,recipients,remote) are refused byconfig set; recipients stay underkey, the remote stays atinit.Security
The review panel caught a reachable prototype-pollution path:
config set security.__proto__.toLocaleString <value>passed the prefix + existence guards (the latter via theinoperator walking the prototype chain) and would have written intoObject.prototypebefore validation. Fixed by:getByPathnow usesObject.hasOwn(own-properties only), so a prototype-walk key reads as absent and is rejected asunknown-keybeforesetByPathruns;setByPathadditionally refuses__proto__/constructor/prototypesegments (defense-in-depth).A regression test asserts the key is refused and
Object.prototypeis untouched.Tests
src/commands/__tests__/config.test.ts(12) + 3 schema back-compat tests: list/get/set, settable-prefix guard, unknown-key + schema-validation rejection, scalar coercion (number/bool/false/enum/array), prototype-pollution refusal, remote-landing assertion (change reaches the bare remote), and diverged-history fail-closed. Full suite: 922 pass / 0 fail.CI note
bun testexits non-zero on the per-file coverage floor; CI treats 0-fail as success.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
configcommand to view and modify vault configuration without editing files directlylist,get, andsetsubcommands with schema validationDocumentation
Tests