Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 8a0808f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b2b481736
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const loaded = await this.localConfig.readPermissionRules(this.cwd); | ||
| this.projectRoot = loaded.projectRoot; | ||
| this.configPath = loaded.configPath; | ||
| this.currentRules = parseProjectPermissionRules(loaded.rules, this.log); |
There was a problem hiding this comment.
When a session is created through the server API or another path that does not first complete the TUI trust prompt, this unconditional read activates rules from an untrusted checkout. A committed .kimi-code/local.toml can therefore contain an allow rule that runs before the normal fallback, sensitive-file, and git-control approval policies, silently relaxing approvals even though project MCP configuration is withheld until trust is granted; inject the workspace trust service and load/unload these rules with trust changes.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| const configRules = this.config.get<PermissionConfig | undefined>(PERMISSION_SECTION)?.rules ?? []; | ||
| return dedupePermissionRules([ | ||
| ...this.agentState.get(permissionRulesKey).rules, | ||
| ...this.projectRules.rules, | ||
| ...configRules, |
There was a problem hiding this comment.
When config.toml contains a valid rule such as decision = "deny", scope = "session-runtime", the new merge exposes it without any diagnostic, but evaluateUserConfiguredRule filters against a scope set containing only turn-override, project, and user. The documented and schema-accepted deny rule is consequently ignored and the tool call can proceed; either include this scope in evaluation or reject it when parsing persistent configuration.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| try { | ||
| await this.fs.mkdir(dirname(configPath), { recursive: true }); | ||
| await this.fs.writeText(configPath, `${stringifyToml(file.raw)}\n`); |
There was a problem hiding this comment.
When an existing local.toml contains comments, adding a project-scoped rule parses the entire file into plain objects and serializes it from scratch here, so every comment and the user's formatting are silently discarded. This file previously had a read-only compatibility role, so users can have hand-maintained annotations; update only the permission table through a comment-preserving TOML writeback instead of replacing the complete document.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| function enqueuePermissionWrite<T>(configPath: string, work: () => Promise<T>): Promise<T> { | ||
| const previous = permissionWriteTails.get(configPath) ?? Promise.resolve(); | ||
| const run = previous.then(work, work); |
There was a problem hiding this comment.
When two Kimi processes use the same checkout, each has its own permissionWriteTails map, so simultaneous appends can both read the same old local.toml and then overwrite one another; whichever write finishes last silently drops the other process's rule. Use the repository's cross-process lock/atomic document mechanism or a locked read-modify-write rather than relying only on this in-memory queue.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| this.log.warn(`project permission rules load failed: ${String(error)}`); | ||
| this.currentRules = []; |
There was a problem hiding this comment.
When local.toml is malformed, unreadable, or temporarily unavailable, this catch resolves session readiness after replacing all project rules with []. A session therefore starts—or a running session continues after reload—without configured deny or ask rules, and users receive only an internal warning; propagate the startup failure and retain the last valid rule set on reload rather than silently removing the restrictions.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| this.log.warn(`failed to persist user permission rules: ${String(error)}`); | ||
| } |
There was a problem hiding this comment.
When the user config is read-only, the disk is full, or a remote project filesystem is unavailable, these catches convert the requested user/project persistence into a warning while addRules has already installed the rule in memory and returns no completion or error signal. Users therefore believe a permanent rule was saved, but it silently disappears after restart; expose an awaitable result or user-visible failure instead of degrading durable scopes to session-only behavior.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| const handle = watchCandidates(this.projectRoot, [this.configPath], { | ||
| ignored: subtreeWatchFilter(this.projectRoot, [this.configPath]), | ||
| }); | ||
| this._register(handle); |
There was a problem hiding this comment.
For local workdirs, every Session-scoped service creates its own watchCandidates handle even when many live sessions point at the same project, and the watch implementation does not deduplicate identical paths. A long-running kap-server or desktop workspace with many open sessions consequently accumulates one OS watcher and duplicate reload work per session, potentially exhausting watcher/file-descriptor limits; attach the watcher to the shared workdir/project service and let sessions consume its rule snapshot.
Useful? React with 👍 / 👎.
| const configRules = this.config.get<PermissionConfig | undefined>(PERMISSION_SECTION)?.rules ?? []; | ||
| return dedupePermissionRules([ | ||
| ...this.agentState.get(permissionRulesKey).rules, | ||
| ...this.projectRules.rules, | ||
| ...configRules, |
There was a problem hiding this comment.
When a persisted rule list uses a specific exception before a broader fallback—for example, allow Bash(git status) followed by deny Bash(git *)—the new merge sends the array to a policy engine that evaluates every deny rule before every allow rule. The broader deny therefore wins despite the documented contract that the first matching [[permission.rules]] entry takes effect; evaluate the merged list once in order rather than partitioning it by decision.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| return dedupePermissionRules([ | ||
| ...this.agentState.get(permissionRulesKey).rules, | ||
| ...this.projectRules.rules, | ||
| ...configRules, |
There was a problem hiding this comment.
After addRules persists a user- or project-scoped rule, the same rule remains permanently in the Agent's live state and is merged ahead of the backing configuration. If the user removes or changes that rule in config.toml or local.toml, the watcher reload succeeds but the stale state copy continues to allow or deny matching calls until the Agent is recreated; remove durable scopes from live-only state after successful persistence or reconcile them when their source reloads.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| try { | ||
| await this.fs.mkdir(dirname(configPath), { recursive: true }); | ||
| await this.fs.writeText(configPath, `${stringifyToml(file.raw)}\n`); |
There was a problem hiding this comment.
On Unix-like systems, a checkout can make .kimi-code/local.toml or its parent directory a symlink outside the project, and this direct writeText follows that link. When the user persists a project rule, Kimi can consequently parse and overwrite an unrelated valid or empty TOML file outside the workspace; resolve the target without following untrusted links, verify containment under the project root, and use a safe atomic replacement.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e04134fa53
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.trusted = await this.trust.get(); | ||
| if (!this.trusted) return; | ||
| const loaded = await this.localConfig.readPermissionRules(this.cwd); |
There was a problem hiding this comment.
When a session uses an SSH, Docker, or command environment, this.cwd and localConfig refer to the remote target, but the injected IWorkspaceTrust still represents the local workspace. Because workspace trust explicitly covers local paths only (docs/en/guides/remote-environment.md:141), trusting the local folder currently causes this code to read the remote checkout's .kimi-code/local.toml without a corresponding trust decision, allowing that checkout to install allow rules and relax approvals. Skip target-side project rules for non-local environments until target-specific trust exists.
Useful? React with 👍 / 👎.
| export const PersistedPermissionRuleSchema = PermissionRuleSchema.extend({ | ||
| scope: z.enum(['project', 'user']).default('user'), | ||
| }); | ||
|
|
||
| export const PermissionConfigSchema = z.object({ | ||
| rules: z.array(PermissionRuleSchema).optional(), | ||
| rules: z.array(PersistedPermissionRuleSchema).optional(), |
There was a problem hiding this comment.
Existing config.toml files could validly contain scope = "turn-override" or scope = "session-runtime" under the previous schema and documentation. Narrowing the persisted schema here, combined with the new unconditional readUserPermissionRules() validation during session materialization, now makes every new or resumed session fail until the user manually edits the file. The earlier silent-ignore case is resolved, but the newly added startup validation is fresh evidence of a compatibility regression; accept and evaluate these scopes or provide a migration rather than rejecting an already valid configuration.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
- load and watch user and project permission rules - persist scoped rule additions without losing concurrent writes - use the active environment's workdir and filesystem for project rules
- keep durable rules out of live agent state and await writes - load trusted project rules at session startup and fail closed - use atomic project writes and update config documentation
e04134f to
2d2e4c0
Compare
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d2e4c04ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }); | ||
|
|
||
| export const PersistedPermissionRuleSchema = PermissionRuleSchema.extend({ | ||
| scope: z.enum(['project', 'user']).default('user'), |
There was a problem hiding this comment.
When ~/.kimi-code/config.toml contains scope = "project", this shared persisted schema accepts it and readUserPermissionRules() subsequently supplies the rule to every agent in every workspace. An allow intended to be project-local can therefore silently relax approvals in unrelated projects; the user-config schema should accept only user, while .kimi-code/local.toml should accept/default only project.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| writes.push(enqueueUserConfigWrite(this.config, async () => { | ||
| const current = this.config.get<PermissionConfig | undefined>(PERMISSION_SECTION); |
There was a problem hiding this comment.
When two CLI or server processes persist user rules concurrently, each process has its own userConfigWriteTails entry and can read the same old permission.rules array before calling replace; the later whole-section write then silently drops the other process's rule. Use a cross-process locked read-modify-write or compare-and-retry mechanism for the shared user config.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| async readPermissionRules(workDir: string): Promise<ProjectPermissionRulesLoadResult> { | ||
| const projectRoot = await this.findProjectRoot(workDir); | ||
| const configPath = this.getProjectLocalConfigPath(projectRoot); | ||
| const file = await this.readProjectLocalToml(configPath); |
There was a problem hiding this comment.
For a trusted managed workspace whose .kimi-code/local.toml contains a malformed legacy workspace.additional_dir, this call goes through readProjectLocalToml(), which validates the unrelated workspace section and rejects session readiness. Managed workspaces deliberately skipped that legacy section before this change, so an otherwise usable workspace can no longer start even when the file contains no permission rules; the permission-loading path should validate only the permission section.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| `permission` sets permission rules that are automatically loaded when a session starts, controlling whether the Agent needs user confirmation before calling a tool. Rules are written as a `[[permission.rules]]` array of tables, matched in order; the first matching rule takes effect. | ||
| `permission` sets permission rules that are automatically loaded when a session starts, controlling whether the Agent needs user confirmation before calling a tool. Rules are written as a `[[permission.rules]]` array of tables. Matching `deny` rules take precedence over matching `ask` rules, which take precedence over matching `allow` rules. | ||
|
|
||
| User rules are loaded from the user `config.toml` at session startup. Project rules are read from `.kimi-code/local.toml` only when the workspace is trusted, with an omitted `scope` defaulting to `project`; project rule changes take effect in a new session and are not hot-reloaded. |
There was a problem hiding this comment.
This newly documented project-rule behavior is backed by code that appends rules to .kimi-code/local.toml, but the same page's project-local section still tells users that local.toml is a read-only legacy file that current versions never write. Users may therefore commit or manage the file under the false assumption that approval choices cannot modify it; update the English and Chinese project-local sections to describe permission-rule writes.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
- ignore legacy runtime scopes in user config without warning - reject project rule persistence in remote environments - update configuration docs and the generated manifest
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a0808fc89
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.cwd = workdir.cwd ?? session.cwd; | ||
| this.local = workdir.environmentId === LOCAL_ENVIRONMENT_ID; | ||
| this.ready = this.local ? this.enqueue(() => this.loadFromDisk()) : Promise.resolve(); |
There was a problem hiding this comment.
When a server or SDK caller creates a session with environment_id: "local" and an environment_cwd outside the registered workspace, this.cwd points at that alternate checkout while this.trust still represents the registered workspace root. Trusting one workspace can therefore load .kimi-code/local.toml from an unrelated, never-trusted local checkout, whose allow rules can silently bypass approvals; derive trust for this.cwd or skip project rules when the local binding directory differs from the trusted root.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
Requirement or Bug
配置系统已经暴露
PermissionRule.scope的user/project持久化语义，但 v2 Agent 没有加载配置中的规则，也不会在addRules时写回。这个 PR 用最小且自洽的支持范围，让用户级和项目级权限规则在会话启动时生效，并在显式写回及新建会话后继续生效。Bug Reproduction Steps
config.toml或受信任项目的.kimi-code/local.toml中配置一条deny规则并创建会话；修复前匹配的工具调用不会按该规则拒绝。addRules添加scope = "user"或scope = "project"的规则；修复前规则仅存在于当前 Agent 内存，重启后丢失。Root Cause
IConfigService和项目本地配置虽然能解析规则，但没有连接到 Agent 的权限规则服务；addRules也只分发非持久化事件。本 PR 通过启动时配置加载和可等待的显式写回完成根本修复。Code Changes
核心设计是持久化规则只以配置文件为数据源。不再把
user/project规则复制进 Agent state，也不做项目热重载，避免配置与内存副本不一致。flowchart LR U[用户 config.toml] --> A[Agent 权限规则合并] T{workspace 已信任?} T -->|是| P[当前环境 .kimi-code/local.toml] T -->|否| N[不加载项目规则] P --> S[Session 启动时项目规则快照] S --> A R[await addRules] -->|user| U R -->|project| Q[串行 + 原子写回] Q --> P A --> G[既有权限策略]addRules现在返回Promise<void>：turn-override/session-runtime仅 dispatch live-only 事件；user/project仅写配置，写回错误由调用方感知。project/user，非法项目规则仍 fail closed。project写回直接报不支持；本地项目根、.kimi-code或local.toml为 symlink 时拒绝写入。阅读顺序：先看
permissionRulesService.ts的运行时/持久化分流，再看projectPermissionRulesService.ts的 trust 与启动加载，最后看projectLocalConfigService.ts的串行原子写回及 Session/Program 的环境接线。Behavior Changes and Affected Users
config.toml中的权限规则user新增规则可 await 写回.kimi-code/local.toml中的权限规则project新增规则写回本地环境；远程不支持turn-override/session-runtime规则addRules写回deny→ask→allow受影响模块为
agent-core-v2的权限规则、项目本地配置、Program workdir 和 Session 生命周期，以及中英文配置文档。没有修改 wire、state 或公开客户端协议。验证结果
agent-core-v2全量：393 个文件、7,760 个测试通过，10 个跳过。此前 trust DI 装配导致的forkParity/sessionTodo失败及新增 Session config stub 问题均已修复。test/app/sessionIndex/sessionIndex.test.ts:1601workspace mtime 断言，与本次权限改动无关。corepack pnpm typecheck：通过，包含 packages 构建及 CLI、VS Code、vis 类型检查。corepack pnpm lint：0 error；lint:imports另有 3 个未触及文件的既有违规。[workspace]、非法项目文件 fail closed，均通过。Checklist
gen-changesetsskill and included the confirmed patch changeset.