feat(permissions): global shell allowlist that cascades with per-project (#2059) by nianyi778 · Pull Request #2091 · esengine/DeepSeek-Reasonix

nianyi778 · 2026-05-28T01:59:05Z

Problem

Closes #2059. Shell command allowlists were per-project only (config.projects[<root>].shellAllowed). There was no way to auto-approve a command (e.g. brew install, cargo build) once and have it apply everywhere — every new project re-prompted. The request: a cascade like Claude/Codex/OpenCode (global ∪ local).

Change

Add a top-level shellAllowedGlobal config list, merged (union) with the per-project list at the single point where the shell tool resolves its extra allowlist:

// src/code/setup.ts
extraAllowed: () => [...new Set([...loadGlobalShellAllowed(), ...loadProjectShellAllowed(root)])]

Manage it via a --global (or -g) flag on the existing /permissions subcommands (works in chat mode too — global entries need no project root):

/permissions add --global <prefix>
/permissions remove --global <prefix-or-N>
/permissions clear --global confirm
/permissions list                          # now shows a Global section above Project

The flag is parsed as a flag, not as a positional keyword, so command prefixes like g (a popular git alias) or global stay manageable from the slash UI.

Config accessors load/add/remove/clearGlobalShellAllowed mirror the project ones (same dedup, exact-match removal, atomic write).
shellAllowedGlobal is registered in STRING_ARRAY_FIELDS so a malformed hand-edited value gets sanitized on read instead of crashing the merge.
i18n: 5 new keys added to EN / zh-CN / ja / de (ru falls back to EN).
The builtin allowlist precedence is unchanged; global is just another source of extraAllowed.

Out of scope (follow-up)

The HTTP /permissions API + desktop settings panel still expose only the project list; global management there can be a follow-up. The /permissions slash command already covers CLI and desktop (command palette).

Verification

tests/config.test.ts: global CRUD (load/add/dedup/remove/clear) + independence from project lists.
tests/permissions-slash.test.ts: --global add/remove/clear routes to the global list; add g adds g to the project list (not hijacked); --global works without a code root.
npm run verify (build + lint + typecheck + full suite) passes.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 26afe9c926

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-28T02:01:50Z

+export function loadGlobalShellAllowed(path: string = defaultConfigPath()): string[] {
+  return readConfig(path).shellAllowedGlobal ?? [];


Validate the global allowlist before returning it

If a user hand-edits config.json with a malformed shellAllowedGlobal value (for example an object or string), this returns that value as string[], and the new callers then spread it in buildCodeToolset and iterate it in /permissions listing. That can throw before any shell command can be checked or before permissions can be listed; readConfig only sanitizes fields included in STRING_ARRAY_FIELDS, and this new field is not added there.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-05-28T02:01:50Z

+  const head = (args[0] ?? "").toLowerCase();
+  if (head === "global" || head === "g") return { global: true, rest: args.slice(1) };


Preserve project prefixes named g or global

With this scope parser, /permissions add g and /permissions remove g are always treated as global-scope operations with no remaining prefix, so an existing or desired project allowlist entry for a command named g (a common git alias) can no longer be managed from the slash command. The same applies to any project command prefix whose first token is global, so users with those project entries are forced out of this UI path.

Useful? React with 👍 / 👎.

esengine

Good direction — closes the right gap (#2059) and the cascade-at-extraAllowed site is the cleanest place to do this. Tests, i18n × 4, and the CRUD mirror are all solid. Two issues to fix before merge (Codex caught both; I agree with both):

Sanitize shellAllowedGlobal on read. The repo already has a registry for exactly this — STRING_ARRAY_FIELDS in src/config.ts (around L409), which readConfig runs through sanitizeStringArrayField. Please add ["shellAllowedGlobal"] to that list. Otherwise a hand-edited config like shellAllowedGlobal: "foo" or shellAllowedGlobal: {} will round-trip as a non-array and crash [...new Set([...loadGlobalShellAllowed(), ...])] in setup.ts before any shell command can be checked.
g / global scope keyword collides with real project entries. takeScope treats the first arg as a scope keyword, so /permissions add g is interpreted as "add to global, empty prefix" and returns the usage hint — there's no way to add g (a popular git alias) to a project allowlist via the slash command. Same for any project entry whose first token is global. Either:
- Use a flag: /permissions add --global <prefix> (parse before joining the rest), or
- Require explicit scope syntax: /permissions add @global <prefix>.
I lean --global — it's the convention readers already expect and won't collide with any plausible prefix.

Once those two are addressed, this is ready. Thanks for hooking into the existing renderListing / load* / add* / remove* / clear* shape — the symmetry made review easy.

nianyi778 · 2026-05-28T02:54:15Z

Both addressed in 625a81b8, thanks for the precise pointers.

Sanitization — added ["shellAllowedGlobal"] to STRING_ARRAY_FIELDS (src/config.ts:412). A malformed value now becomes [] on read instead of reaching the spread in setup.ts.
Flag, not keyword — takeScope now looks for --global / -g anywhere in the args and strips it; /permissions add g falls through as a normal project prefix. Updated the four i18n strings that printed the old add global … syntax. Three new tests cover --global add/remove/clear, add g going to project (not global), and --global working without a code root.

nianyi778 · 2026-05-28T05:01:25Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 625a81b831

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-28T05:04:04Z

+  for (const a of args) {
+    if (a === "--global" || a === "-g") global = true;
+    else rest.push(a);


Restrict --global parsing to the option position

Because this strips --global/-g from every argument after the subcommand, a user cannot add or remove project allowlist prefixes for real commands that contain those flags, such as /permissions add git config --global user.name or npm install --global foo; the handler instead switches to the global list and stores a different prefix with the flag removed, so the intended command still prompts while an unintended prefix may be globally allowed. Parse the scope flag only before the prefix (or provide a -- terminator) so command arguments are preserved.

Useful? React with 👍 / 👎.

nianyi778 · 2026-05-28T05:19:11Z

Fixed in a683c97d. takeScope now only consumes --global / -g from the leading option position and stops at the first non-flag, so an inner --global is preserved verbatim:

/permissions add git config --global user.name → adds the full prefix git config --global user.name to project (no scope hijack).
/permissions add --global npm install --global foo → adds npm install --global foo to global (leading flag consumed, inner flag preserved).

Two new tests cover both cases.

nianyi778 · 2026-05-28T05:27:27Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a683c97db8

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-28T05:30:24Z

-      extraAllowed: () => loadProjectShellAllowed(root),
+      // Global allowlist applies everywhere; project list adds to it (#2059).
+      extraAllowed: () => [
+        ...new Set([...loadGlobalShellAllowed(), ...loadProjectShellAllowed(root)]),


Read the global allowlist from the selected config

When buildCodeToolset is constructed with opts.configPath (the option is documented for pinning a non-default config, and is already used for loadEditMode(opts.configPath)), the new global allowlist still reads ~/.reasonix/config.json via loadGlobalShellAllowed() instead of the selected config file. In that context, entries saved in the active config's shellAllowedGlobal will not auto-approve commands, while entries from the user's default config may be applied unexpectedly; pass opts.configPath through when loading the new global list.

Useful? React with 👍 / 👎.

nianyi778 · 2026-05-28T05:42:29Z

Fixed in e77f6b4e. Passed opts.configPath through to all three config calls inside registerRooted:

readConfig(opts.configPath)
loadGlobalShellAllowed(opts.configPath)
loadProjectShellAllowed(root, opts.configPath)
loadEditMode(opts.configPath) (the allowAll getter had the same gap)

@codex review

chatgpt-codex-connector · 2026-05-28T05:42:41Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

nianyi778 · 2026-05-29T05:00:45Z

@esengine 您好，您在 2026-05-28 02:16 提出的两条修改建议（STRING_ARRAY_FIELDS 净化注册 + --global flag 不劫持内部参数）已全部修完，共经历 3 轮迭代，当前 head e77f6b4 是最终状态。Codex 当天额度耗尽未能重审最新 commit，但之前几轮均通过。麻烦您抽空复审一下，感谢 🙏

esengine

Re-reviewed — both issues from my review are fixed exactly as discussed:

["shellAllowedGlobal"] is now in STRING_ARRAY_FIELDS, so a hand-edited non-array value gets sanitized on read instead of crashing the [...new Set(...)] cascade in setup.ts.
Scope is now a leading --global/-g flag via takeScope (not a positional keyword) — so g is addable to a project allowlist again, a non-leading --global (e.g. git config --global user.name) is preserved, and the i18n usage hints updated to --global. That's the option I leaned toward.
CI green. Merging — thanks for hooking into the existing field-registry + CRUD shape.

esengine · 2026-05-29T05:33:05Z

Approving the content — both fixes are verified correct. But heads up: it now conflicts with main (recent merges touched src/config.ts/setup.ts). Please rebase onto current main (git fetch origin main && git merge origin/main) and I'll merge — the review stands, no further changes needed beyond the conflict resolution.

…ect (esengine#2059) Adds a top-level `shellAllowedGlobal` config list, auto-approved across ALL projects and merged (union) with the per-project allowlist when deciding whether a shell command needs approval. Manage it via a `--global` (or `-g`) flag on the existing `/permissions` subcommands: /permissions add --global <prefix> /permissions remove --global <prefix-or-N> /permissions clear --global confirm /permissions list # now shows a Global section above Project The flag is parsed as a flag (not a positional keyword), so command prefixes like `g` or `global` remain manageable from the slash UI. `shellAllowedGlobal` is registered in `STRING_ARRAY_FIELDS` so a malformed hand-edited value is sanitized on read instead of crashing the merge in `code/setup.ts`'s `extraAllowed` getter. Config accessors (load/add/remove/clearGlobalShellAllowed) mirror the project ones; EN/zh-CN/ja/de strings added (ru falls back to EN).

nianyi778 · 2026-05-29T05:49:16Z

Rebased onto current main (81fd7872) — resolved the conflict in src/code/setup.ts (kept both autoGitRollback: {} from HEAD and opts.configPath threading from this PR) and in tests/config.test.ts (kept both promptHistory and globalShellAllowed test blocks).

nianyi778 · 2026-05-29T06:13:20Z

Windows CI 失败的是 auto-git-rollback.test.ts，超时（5000ms），Ubuntu 构建通过 ✅。这是 Windows 上 git 操作偶发超时的 flaky test，与本 PR 的改动（config.ts / setup.ts / permissions.ts）无关。

chatgpt-codex-connector Bot reviewed May 28, 2026

View reviewed changes

esengine requested changes May 28, 2026

View reviewed changes

nianyi778 force-pushed the feat/global-shell-allowlist branch from 26afe9c to 625a81b Compare May 28, 2026 02:53

chatgpt-codex-connector Bot reviewed May 28, 2026

View reviewed changes

nianyi778 force-pushed the feat/global-shell-allowlist branch from 625a81b to a683c97 Compare May 28, 2026 05:18

chatgpt-codex-connector Bot reviewed May 28, 2026

View reviewed changes

nianyi778 force-pushed the feat/global-shell-allowlist branch from a683c97 to e77f6b4 Compare May 28, 2026 05:42

esengine approved these changes May 29, 2026

View reviewed changes

nianyi778 force-pushed the feat/global-shell-allowlist branch from e77f6b4 to 81fd787 Compare May 29, 2026 05:47

esengine merged commit 744d1f9 into esengine:main May 29, 2026
5 of 6 checks passed

		export function loadGlobalShellAllowed(path: string = defaultConfigPath()): string[] {
		return readConfig(path).shellAllowedGlobal ?? [];

		const head = (args[0] ?? "").toLowerCase();
		if (head === "global" \|\| head === "g") return { global: true, rest: args.slice(1) };

Conversation

nianyi778 commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Out of scope (follow-up)

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

esengine left a comment

Choose a reason for hiding this comment

Uh oh!

nianyi778 commented May 28, 2026

Uh oh!

nianyi778 commented May 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

nianyi778 commented May 28, 2026

Uh oh!

nianyi778 commented May 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

nianyi778 commented May 28, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 28, 2026

Uh oh!

nianyi778 commented May 29, 2026

Uh oh!

esengine left a comment

Choose a reason for hiding this comment

Uh oh!

esengine commented May 29, 2026

Uh oh!

nianyi778 commented May 29, 2026

Uh oh!

nianyi778 commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nianyi778 commented May 28, 2026 •

edited

Loading