fix: argument denylist + Windows shell escaping#536
Open
openasocket wants to merge 195 commits intoRunMaestro:0.16.0-RCfrom
Open
fix: argument denylist + Windows shell escaping#536openasocket wants to merge 195 commits intoRunMaestro:0.16.0-RCfrom
openasocket wants to merge 195 commits intoRunMaestro:0.16.0-RCfrom
Conversation
|
Important Review skippedToo many files! This PR contains 231 files, which is 81 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (231)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request introduces Windows shell decision logic optimization and custom argument validation. It adds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR adds two security hardening features: a Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Windows agent spawn request] --> B{canRunWithoutShell\ncommandToSpawn?}
B -- "true (.exe with path)" --> C[useShell = false\nspawn directly\nno shell metachar risk]
B -- "false (.cmd/.bat/bare name)" --> D[buildExpandedEnv\ncustomEnvVarsToPass]
D --> E[getWindowsShellForAgentExecution]
E --> F{Shell priority}
F -- "1 · customShellPath" --> G[useShell = true\nshell = custom]
F -- "2 · currentShell ≠ cmd.exe" --> H[useShell = true\nshell = current]
F -- "3 · default" --> I[useShell = true\nshell = PowerShell]
C --> J[spawn process]
G & H & I --> K[escapeArgsForShell\nargs]
K --> J
subgraph "agent-args.ts — before spawn"
L[parseCustomArgs\nsessionCustomArgs] --> M{DENIED_CUSTOM_ARGS\n.has arg?}
M -- "exact match ✓" --> N[warn + strip]
M -- "flag=value bypass ✗" --> O[passes through\n⚠ security gap]
M -- "clean arg" --> P[keep]
end
Last reviewed commit: b0cbd15 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+167
to
+173
| const filteredCustomArgs = parsedCustomArgs.filter((arg) => { | ||
| if (DENIED_CUSTOM_ARGS.has(arg)) { | ||
| logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg }); | ||
| return false; | ||
| } | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Denylist bypass via flag=value syntax
The filter uses DENIED_CUSTOM_ARGS.has(arg), which performs an exact string match on each parsed token. A user can bypass every entry on the denylist by attaching a value with =:
--approval-mode=bypassPermissions→ token is"--approval-mode=bypassPermissions"→ not in the Set → passes through--dangerously-skip-permissions=true→ same bypass--dangerous-auto-approve=1→ same bypass
--approval-mode in particular is designed to accept a value, making this an exploitable path.
The fix is to also block any token that starts with a denied flag followed by =:
const filteredCustomArgs = parsedCustomArgs.filter((arg) => {
const isDenied =
DENIED_CUSTOM_ARGS.has(arg) ||
[...DENIED_CUSTOM_ARGS].some((denied) => arg.startsWith(`${denied}=`));
if (isDenied) {
logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg });
return false;
}
return true;
});
Comment on lines
+331
to
+340
| if (canRunWithoutShell(commandToSpawn)) { | ||
| // Command is a fully-resolved .exe path — spawn directly without shell. | ||
| // This is the safest option: no shell metacharacter interpretation at all. | ||
| // Note: .cmd/.bat files (e.g., npm-installed CLIs like Gemini CLI) | ||
| // cannot use this path — they require shell interpretation. | ||
| useShell = false; | ||
| logger.info(`Using shell:false for resolved .exe on Windows`, LOG_CONTEXT, { | ||
| agentId: agent?.id, | ||
| command: commandToSpawn, | ||
| }); |
There was a problem hiding this comment.
buildExpandedEnv skipped for .exe spawns — env vars not pre-expanded
In the original code, buildExpandedEnv(customEnvVarsToPass) was always called on Windows before spawning. This PR moves it into the else branch, so it is now skipped whenever canRunWithoutShell returns true.
When spawning with shell: false, Node.js passes env vars directly to the OS. Any custom env var whose value contains references like %SOME_VAR% or $SOME_VAR will not be expanded because there is no shell to do it. buildExpandedEnv exists precisely to pre-expand those references, so skipping it for .exe spawns means user-configured env vars that reference other variables will be passed in raw, unexpanded form.
Consider calling buildExpandedEnv before the canRunWithoutShell branch to keep the pre-expansion behaviour consistent:
// Always expand env vars on Windows before spawning (shell or no shell)
const expandedEnv = buildExpandedEnv(customEnvVarsToPass);
customEnvVarsToPass = Object.fromEntries(
Object.entries(expandedEnv).filter(([_, value]) => value !== undefined)
) as Record<string, string>;
if (canRunWithoutShell(commandToSpawn)) {
useShell = false;
// ...
} else {
// shell selection ...
}
Comment on lines
+159
to
+166
| export function canRunWithoutShell(command: string): boolean { | ||
| const path = require('path'); | ||
| const ext = path.extname(command).toLowerCase(); | ||
| const hasPath = /\\|\//.test(command); | ||
|
|
||
| // Must have a directory path (not just a basename) and be a native .exe | ||
| return hasPath && ext === '.exe'; | ||
| } |
There was a problem hiding this comment.
require('path') inside function body
canRunWithoutShell calls require('path') at the call site instead of importing it at the module top level. The same pattern exists for require('fs') in getWindowsShellForAgentExecution. While Node.js caches require so there is no runtime cost, this is inconsistent with the ES-module import style used throughout the rest of the codebase and makes it harder to statically analyze or tree-shake the module.
Consider adding a top-level import:
import path from 'path';and removing the inline require('path') call.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/main/process-manager/utils/shellEscape.test.ts`:
- Around line 115-118: Update the test for empty-string handling: change the
expectation for escapeCmdArg('') to expect '""' (an explicitly quoted empty
string) because cmd.exe drops unquoted empty args, and add a new test asserting
escapePowerShellArg('') returns '' (an empty string) to cover PowerShell
behavior; locate and modify the tests referencing escapeCmdArg and add one for
escapePowerShellArg in the existing shellEscape.test.ts so they reflect these
correct platform-specific outputs.
In `@src/main/ipc/handlers/process.ts`:
- Around line 330-331: The Windows-shell skip currently checks
config.sessionSshRemoteConfig?.enabled before resolving SSH, causing shell/PATH
setup to be skipped even when getSshRemoteConfig() returns no usable config;
change the logic in the process spawn path (around isWindows and
canRunWithoutShell) to call getSshRemoteConfig() first and validate the returned
config (e.g., non-null and has a valid remoteId or matches the terminal request)
before trusting config.sessionSshRemoteConfig?.enabled; if getSshRemoteConfig()
yields no usable remote, fall back to the Windows shell/PATH handling (i.e.,
only skip shell when a resolved, usable SSH config exists), and apply the same
adjustment to the other spawn block referenced (the 389–403 region) so both
branches consistently resolve SSH before deciding to skip the Windows shell.
In `@src/main/utils/agent-args.ts`:
- Around line 10-18: The denylist currently only checks exact tokens against
DENIED_CUSTOM_ARGS so forms like "--approval-mode=full-auto" slip through and
"full-auto" can become an orphaned positional; update the token-filtering logic
that processes agent args to (1) normalize tokens by splitting at the first '='
for long options before checking against DENIED_CUSTOM_ARGS, and (2) when a
denied flag that accepts a value is encountered (either as "--flag=value" or
"--flag" followed by a separate token), ensure you consume/remove the value
token as well so no orphan remains; keep DENIED_CUSTOM_ARGS as the canonical set
and apply these checks in the same function that iterates/filters args so both
"--flag=value" and "--flag value" are blocked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a28b537d-456a-4067-ad21-df920296bac8
📒 Files selected for processing (6)
src/__tests__/main/process-manager/utils/shellEscape.test.tssrc/__tests__/main/utils/agent-args.test.tssrc/main/ipc/handlers/process.tssrc/main/process-manager/utils/index.tssrc/main/process-manager/utils/shellEscape.tssrc/main/utils/agent-args.ts
Comment on lines
+115
to
+118
| it('should handle empty string', () => { | ||
| // Empty string has no special chars and is short, returned as-is | ||
| expect(escapeCmdArg('')).toBe(''); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the implementation file for escapeCmdArg
fd -t f "shellEscape" --extension ts --extension js --extension tsx --extension jsxRepository: RunMaestro/Maestro
Length of output: 168
🏁 Script executed:
# Look at the test file structure to understand context
cat -n src/__tests__/main/process-manager/utils/shellEscape.test.ts | head -150Repository: RunMaestro/Maestro
Length of output: 6276
🏁 Script executed:
# Search for escapeCmdArg and escapePowerShellArg implementations
rg "escapeCmdArg|escapePowerShellArg" -A 15 --type ts --type tsx | head -100Repository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
# Get the implementation of escapeCmdArg and related functions
cat -n src/main/process-manager/utils/shellEscape.tsRepository: RunMaestro/Maestro
Length of output: 12971
🏁 Script executed:
# Check if there are PowerShell empty string tests
rg "escapePowerShellArg.*''" src/__tests__/main/process-manager/utils/shellEscape.test.ts -A 5Repository: RunMaestro/Maestro
Length of output: 322
🏁 Script executed:
# Check how escapeCmdArg and escapePowerShellArg are used in the codebase
rg "escapeCmdArg|escapePowerShellArg" --type ts -B 3 -A 3 | head -200Repository: RunMaestro/Maestro
Length of output: 18692
🏁 Script executed:
# Look for tests specifically for empty string handling in PowerShell
cat -n src/__tests__/main/process-manager/utils/shellEscape.test.ts | sed -n '132,300p'Repository: RunMaestro/Maestro
Length of output: 7317
🏁 Script executed:
# Check if there are any tests for PowerShell empty string
rg "escapePowerShellArg.*empty|empty.*escapePowerShellArg" -i src/__tests__/main/process-manager/utils/shellEscape.test.ts -A 5Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
# Let me check the full PowerShell test suite to see if there's an empty string test
tail -n +132 src/__tests__/main/process-manager/utils/shellEscape.test.ts | head -70Repository: RunMaestro/Maestro
Length of output: 2408
Empty strings must be quoted to survive shell argument parsing.
The expectation escapeCmdArg('') === '' is incorrect. In cmd.exe, an empty unquoted argument is parsed away—it must be "" to be preserved as an actual argument token. Similarly, escapePowerShellArg('') should return '', but there's no test for this case.
🧪 Suggested fix
- it('should handle empty string', () => {
- // Empty string has no special chars and is short, returned as-is
- expect(escapeCmdArg('')).toBe('');
- });
+ it('should preserve empty string arguments', () => {
+ expect(escapeCmdArg('')).toBe('""');
+ expect(escapePowerShellArg('')).toBe("''");
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/main/process-manager/utils/shellEscape.test.ts` around lines
115 - 118, Update the test for empty-string handling: change the expectation for
escapeCmdArg('') to expect '""' (an explicitly quoted empty string) because
cmd.exe drops unquoted empty args, and add a new test asserting
escapePowerShellArg('') returns '' (an empty string) to cover PowerShell
behavior; locate and modify the tests referencing escapeCmdArg and add one for
escapePowerShellArg in the existing shellEscape.test.ts so they reflect these
correct platform-specific outputs.
src/main/ipc/handlers/process.ts
Outdated
Comment on lines
+330
to
+331
| if (isWindows && !config.sessionSshRemoteConfig?.enabled) { | ||
| // Use expanded environment with custom env vars to ensure PATH includes all binary locations | ||
| const expandedEnv = buildExpandedEnv(customEnvVarsToPass); | ||
| // Filter out undefined values to match Record<string, string> type | ||
| customEnvVarsToPass = Object.fromEntries( | ||
| Object.entries(expandedEnv).filter(([_, value]) => value !== undefined) | ||
| ) as Record<string, string>; | ||
|
|
||
| // Get the preferred shell for Windows (custom -> current -> PowerShell) | ||
| // PowerShell is preferred over cmd.exe to avoid command line length limits | ||
| const customShellPath = settingsStore.get('customShellPath', '') as string; | ||
| const shellConfig = getWindowsShellForAgentExecution({ | ||
| customShellPath, | ||
| currentShell: shellToUse, | ||
| }); | ||
| shellToUse = shellConfig.shell; | ||
| useShell = shellConfig.useShell; | ||
|
|
||
| logger.info(`Forcing shell execution for agent on Windows for PATH access`, LOG_CONTEXT, { | ||
| agentId: agent?.id, | ||
| command: commandToSpawn, | ||
| args: argsToSpawn, | ||
| shell: shellToUse, | ||
| shellSource: shellConfig.source, | ||
| }); | ||
| if (canRunWithoutShell(commandToSpawn)) { |
There was a problem hiding this comment.
Resolve SSH before deciding to skip the Windows shell path.
This guard keys off sessionSshRemoteConfig.enabled, not on a successful getSshRemoteConfig() result. If enabled is true but remoteId is null/stale—or if a terminal request carries this field—we skip the Windows shell/PATH setup and then fall through to a local shell:false spawn. On Windows that breaks .cmd wrappers and other PATH-dependent agents; please move the Windows fallback after SSH resolution, or rerun it when no usable remote config is found.
Also applies to: 389-403
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/ipc/handlers/process.ts` around lines 330 - 331, The Windows-shell
skip currently checks config.sessionSshRemoteConfig?.enabled before resolving
SSH, causing shell/PATH setup to be skipped even when getSshRemoteConfig()
returns no usable config; change the logic in the process spawn path (around
isWindows and canRunWithoutShell) to call getSshRemoteConfig() first and
validate the returned config (e.g., non-null and has a valid remoteId or matches
the terminal request) before trusting config.sessionSshRemoteConfig?.enabled; if
getSshRemoteConfig() yields no usable remote, fall back to the Windows
shell/PATH handling (i.e., only skip shell when a resolved, usable SSH config
exists), and apply the same adjustment to the other spawn block referenced (the
389–403 region) so both branches consistently resolve SSH before deciding to
skip the Windows shell.
Comment on lines
+10
to
+18
| const DENIED_CUSTOM_ARGS = new Set([ | ||
| '--no-sandbox', | ||
| '--include-directories', | ||
| '--dangerous-auto-approve', | ||
| '--dangerously-skip-permissions', | ||
| '--dangerously-bypass-approvals-and-sandbox', | ||
| '--approval-mode', | ||
| '-y', | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/utils/agent-args.ts | head -50Repository: RunMaestro/Maestro
Length of output: 1745
🏁 Script executed:
cat -n src/main/utils/agent-args.ts | sed -n '160,180p'Repository: RunMaestro/Maestro
Length of output: 882
🏁 Script executed:
# Find parseCustomArgs function
rg -n "parseCustomArgs" src/main/utils/agent-args.ts -A 15Repository: RunMaestro/Maestro
Length of output: 1233
The denylist still misses --flag=value forms and leaves orphan values.
The filter at lines 167-173 only rejects exact token matches. --approval-mode=full-auto passes through unchanged, and --approval-mode full-auto strips only the flag, leaving full-auto as a positional argument. The filtering needs to normalize long options on = and consume the following token for denied flags that take a value.
🔧 Possible shape of the fix
const DENIED_CUSTOM_ARGS = new Set([
'--no-sandbox',
'--include-directories',
'--dangerous-auto-approve',
'--dangerously-skip-permissions',
'--dangerously-bypass-approvals-and-sandbox',
'--approval-mode',
'-y',
]);
+const DENIED_CUSTOM_ARGS_WITH_VALUE = new Set([
+ '--include-directories',
+ '--approval-mode',
+]);
const parsedCustomArgs = parseCustomArgs(effectiveCustomArgs);
-const filteredCustomArgs = parsedCustomArgs.filter((arg) => {
- if (DENIED_CUSTOM_ARGS.has(arg)) {
- logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg });
- return false;
- }
- return true;
-});
+const filteredCustomArgs: string[] = [];
+for (let i = 0; i < parsedCustomArgs.length; i++) {
+ const arg = parsedCustomArgs[i];
+ const normalizedArg = arg.split('=')[0];
+ if (DENIED_CUSTOM_ARGS.has(normalizedArg)) {
+ logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg: normalizedArg });
+ if (DENIED_CUSTOM_ARGS_WITH_VALUE.has(normalizedArg) && !arg.includes('=') && i + 1 < parsedCustomArgs.length) {
+ i += 1;
+ }
+ continue;
+ }
+ filteredCustomArgs.push(arg);
+}Also applies to: 167-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/utils/agent-args.ts` around lines 10 - 18, The denylist currently
only checks exact tokens against DENIED_CUSTOM_ARGS so forms like
"--approval-mode=full-auto" slip through and "full-auto" can become an orphaned
positional; update the token-filtering logic that processes agent args to (1)
normalize tokens by splitting at the first '=' for long options before checking
against DENIED_CUSTOM_ARGS, and (2) when a denied flag that accepts a value is
encountered (either as "--flag=value" or "--flag" followed by a separate token),
ensure you consume/remove the value token as well so no orphan remains; keep
DENIED_CUSTOM_ARGS as the canonical set and apply these checks in the same
function that iterates/filters args so both "--flag=value" and "--flag value"
are blocked.
Collaborator
Maintainer ReviewGood direction on both the denylist and Must Fix1. Denylist bypass via The Fix: split on 2.
3. Merge conflict This PR currently has merge conflicts with Nice to Have
SummaryPlease rebase onto |
Introduce maestro:// URL scheme support for navigating to agents, tabs, and groups from external apps and OS notification clicks. - Add deep-links module with URL parsing, protocol registration, single-instance locking, and cross-platform event handling - Wire notification click handlers to navigate to the originating agent/tab via deep link dispatch - Thread sessionId/tabId context through notification preload bridge - Add onDeepLink listener in renderer with routing to existing navigation handlers - Register maestro:// protocol in electron-builder config - Add 18 tests covering URL parsing and notification click wiring
- URI-encode sessionId/tabId when constructing deep link URLs in notification click handler to prevent malformed URLs with special chars - Add process.exit(0) after app.quit() so secondary instances exit immediately without running further module-level setup - Use useRef for sessions in deep link effect to avoid tearing down and re-registering the IPC listener on every sessions change - Guard against navigating to non-existent session IDs in deep link handler to prevent invalid UI state - Add cross-reference comment in global.d.ts linking to canonical ParsedDeepLink type (can't import in ambient declaration file) - Add test for URI-encoding round-trip in notification click handler
- Add shared deep-link-urls.ts with buildSessionDeepLink(),
buildGroupDeepLink(), and buildFocusDeepLink() utilities
- Add {{AGENT_DEEP_LINK}}, {{TAB_DEEP_LINK}}, {{GROUP_DEEP_LINK}}
template variables available in system prompts, custom AI commands,
and Auto Run documents
- Wire activeTabId and groupId into TemplateContext at all call sites
(agentStore, useInputProcessing, useRemoteHandlers,
useDocumentProcessor, useMergeTransferHandlers, batch-processor)
- Refactor notifications.ts to use shared buildSessionDeepLink()
- Add sessionId/tabId to notifyToast callers where context is available
(merge, transfer, summarize, PR creation)
- Add docs/deep-links.md documentation page with URL format, usage
examples, template variables, and platform behavior
- Add 8 tests for URL builders, 6 tests for template variable
substitution including URI encoding
…e activity indicators Broadcast new history entries via IPC when they are added, subscribe in the UnifiedHistoryTab with RAF batching and deduplication, and extend the HistoryStatsBar with spinning Active agent count and Queued message count indicators derived from the Zustand session store.
…orrect types - Replace unstable sessionNameMap Zustand selector (new Map per render) with a stable ref + subscribe pattern to avoid streaming effect re-subscription - Dedupe within batch before merging; compute setTotalEntries and setHistoryStats from deduplicated entries only (not raw batch) - Clear pendingEntriesRef on cleanup to prevent stale replay after resubscribe - Use HistoryEntry (not UnifiedHistoryEntry) in preload callback type since the wire payload lacks sourceSessionId - Use canonical UsageStats interface in global.d.ts (fixes pre-existing cacheReadTokens/cacheWriteTokens field name mismatch)
…URL registry support - Add `symphony: boolean` (default true) to EncoreFeatureFlags - Gate Symphony modal, menu item, keyboard shortcut (⇧⌘Y), and command palette entry - Add `symphonyRegistryUrls` setting for user-configured additional registry URLs - Replace single `fetchRegistry()` with `fetchRegistries()` that fetches default + custom URLs in parallel - Merge repositories by slug (default registry wins on conflicts), isolated per-URL error handling - Add Symphony toggle + Registry Sources UI in Settings > Encore tab - Update tests for new symphony flag across all encore feature assertions
- Redact registry URLs before logging to prevent credential leakage - Skip registry cache when custom source URLs are configured (stale cache fix) - Runtime-validate symphonyRegistryUrls from settings store - Reset modal-open flags when Encore Feature toggles are disabled - Normalize registry URLs before duplicate/default checks - Add aria-label to icon-only registry URL remove button - Expose setSymphonyRegistryUrls in getSettingsActions() - Validate persisted symphonyRegistryUrls with Array.isArray guard
…r, and Encore feature flag - Register maestroCue as an Encore Feature flag (EncoreFeatureFlags, DEFAULT_ENCORE_FEATURES) - Create src/main/cue/cue-types.ts with CueEventType, CueSubscription, CueSettings, CueConfig, CueEvent, CueRunStatus, CueRunResult, CueSessionStatus, and related constants - Add 'CUE' to HistoryEntryType across shared types, global.d.ts, preload, IPC handlers, and hooks - Add cueTriggerName, cueEventType, cueSourceSession optional fields to HistoryEntry - Add 'cue' log level to MainLogLevel, LOG_LEVEL_PRIORITY, logger switch/case, and LogViewer with teal color (#06b6d4), always-enabled filter, and agent name pill - Add 10 Cue-specific template variables (CUE_EVENT_TYPE, CUE_TRIGGER_NAME, etc.) with cueOnly flag - Extend TemplateContext with cue? field and substituteTemplateVariables with Cue replacements - Update TEMPLATE_VARIABLES_GENERAL filter to exclude cueOnly variables
…ovider Implements the three core modules for the Cue event-driven automation engine: - cue-yaml-loader.ts: Discovers and parses maestro-cue.yaml files with js-yaml, validates config structure, watches for file changes via chokidar with 1-second debounce - cue-file-watcher.ts: Wraps chokidar for file.changed subscriptions with per-file debouncing (5s default), constructs CueEvent instances with full file metadata payloads - cue-engine.ts: Main coordinator class with dependency injection, manages time.interval timers (fires immediately then on interval), file watchers, agent.completed listeners with fan-in tracking, activity log ring buffer (max 500), and run lifecycle management Added js-yaml and @types/js-yaml dependencies. 57 tests across 3 test files.
…story recording Implements the Cue executor module that spawns background agent processes when Cue triggers fire, following the same spawn pattern as Auto Run's process:spawn IPC handler. Key exports: - executeCuePrompt(): Full 10-step pipeline (prompt resolution, template substitution, agent arg building, SSH wrapping, process spawn with stdout/stderr capture, timeout enforcement with SIGTERM→SIGKILL) - stopCueRun(): Graceful process termination by runId - recordCueHistoryEntry(): Constructs HistoryEntry with type 'CUE' and all Cue-specific fields (trigger name, event type, source session) - getActiveProcesses(): Monitor running Cue processes Test coverage: 31 tests in cue-executor.test.ts covering execution paths, SSH remote, timeout escalation, history entry construction, and edge cases. Full suite: 21,635 tests passing across 512 files, zero regressions.
…zation for Maestro Cue
Add CUE entry support across all History components: - HistoryFilterToggle: CUE filter button with teal (#06b6d4) color and Zap icon - HistoryEntryItem: CUE pill, success/failure badges, and trigger metadata subtitle - HistoryPanel & UnifiedHistoryTab: CUE included in default activeFilters - HistoryDetailModal: CUE pill color, icon, success/failure indicator, trigger metadata display - Comprehensive test coverage for all CUE rendering paths (205 new/updated tests pass)
…nd activity log Add the Maestro Cue dashboard modal with full Encore Feature gating: - CueModal component with sessions table, active runs list, and activity log - useCue hook for state management, event subscriptions, and 10s polling - Settings toggle in Encore tab, command palette entry, keyboard shortcut (Cmd+Shift+U) - SessionList hamburger menu entry, modal store integration, lazy loading - 30 tests covering hook behavior and modal rendering
Add CueYamlEditor component for creating and editing maestro-cue.yaml files. Features split-view layout with AI assist (left panel for description + clipboard copy) and YAML editor (right panel with line numbers, debounced validation, Tab indentation). Integrates into CueModal via Edit YAML button on each session row.
…yaml Task 1: CueHelpModal component with 7 content sections (What is Maestro Cue, Getting Started, Event Types, Template Variables, Multi-Agent Orchestration, Timeouts & Failure Handling, AI YAML Editor). Wired to CueModal ? button. Registered with layer stack at MODAL_PRIORITIES.CUE_HELP (465). Task 2: useCueAutoDiscovery hook that calls cue:refreshSession when sessions are created/restored/removed, gated by encoreFeatures.maestroCue. Full scan on feature enable, engine disable on feature off. Tests: 38 CueHelpModal tests + 10 useCueAutoDiscovery tests, all passing. Lint clean. No existing test regressions (21,778 tests pass).
… session bridging Implement agent completion event chaining in the Cue engine: - Fan-out: subscriptions dispatch prompts to multiple target sessions simultaneously - Fan-in: subscriptions wait for all source sessions to complete before firing, with timeout handling (break clears tracker, continue fires with partial data) - Session bridging: user session completions trigger Cue subscriptions via exit listener - Add AgentCompletionData type for rich completion event payloads - Add hasCompletionSubscribers() optimization to skip unneeded notifications - Wire getCueEngine/isCueEnabled into ProcessListenerDependencies
…ure gated) Add teal Zap icon next to session names in the Left Bar for sessions with active Maestro Cue subscriptions. The indicator is gated behind the maestroCue Encore Feature flag and shows a tooltip with the subscription count on hover. - Add cueSubscriptionCount prop to SessionItem with Zap icon rendering - Add lightweight Cue status fetching in SessionListInner via cue:getStatus IPC, refreshed on cue:activityUpdate events - Add cue namespace to global test setup mock - 6 unit tests + 3 integration tests; all 21,815 tests pass; lint clean
Add Maestro Cue entries across all developer documentation: - CLAUDE.md: Key Files table (4 entries), Architecture tree (cue/ dir), Standardized Vernacular (Cue + Cue Modal terms) - CLAUDE-PATTERNS.md: Encore Feature section lists maestroCue as second reference implementation alongside directorNotes - CLAUDE-IPC.md: cue namespace in Automation section, full Cue API reference with all endpoints and event documentation
…t journal - Add cue-db.ts: SQLite-backed event journal (cue_events table) and single-row heartbeat table (cue_heartbeat) using better-sqlite3 with WAL mode - Add cue-reconciler.ts: time event catch-up logic that fires exactly one reconciliation event per missed subscription (no flooding), with payload.reconciled and payload.missedCount metadata - Update cue-engine.ts: heartbeat writer (30s interval), sleep detection (2-minute gap threshold), database pruning (7 days), and clean shutdown - Update CueHelpModal: new "Sleep & Recovery" section with Moon icon - Update CueModal: amber "catch-up" badge on reconciled activity log entries - Tests: 41 new tests across cue-db (17), cue-reconciler (11), cue-sleep-wake (13)
Add filter field to CueSubscription for narrowing when subscriptions fire. Supports exact match, negation (!), numeric comparison (>/</>=/<=), glob patterns (picomatch), and boolean matching with AND logic. Filter checks integrated at all three dispatch points (file.changed, time.interval, agent.completed). Includes help modal docs, AI prompt updates, and 80 new tests (43 filter engine + 37 YAML loader).
… awareness Add pattern presets (Scheduled Task, File Enrichment, Reactive, Research Swarm, Sequential Chain, Debate) to the YAML editor as clickable cards. Enhance the AI system prompt with pattern recognition guidance. Add a Coordination Patterns section with ASCII flow diagrams to the help modal.
Add github.pull_request and github.issue event types to CueEventType union. Add repo and poll_minutes fields to CueSubscription interface. Add cue_github_seen SQLite table with 5 CRUD functions for tracking seen GitHub items (isGitHubItemSeen, markGitHubItemSeen, hasAnyGitHubSeen, pruneGitHubSeen, clearGitHubSeenForSubscription). Create cue-github-poller.ts module that polls GitHub CLI for new PRs/issues, seeds existing items on first run, and fires CueEvents for new items. Comprehensive test suite with 17 test cases covering all polling behaviors. All 264 Cue tests pass, lint clean.
- Document Graph controls now auto-close other dropdowns for cleaner UX 🧭 - Session bookmarks hide automatically when filtering unread agents only 🔖 - Unread tab filter now keeps busy tabs visible for better awareness ⏳ - Tab keyboard shortcuts now work across AI, terminal, and file modes ⌨️ - Unread-only navigation now treats busy tabs as navigable targets 🧠
When showUnreadAgentsOnly is active, agent cycling (Cmd+[/]) now only visits unread or busy agents instead of all visible agents. The currently active agent is always included so you don't get stuck. Claude ID: 21220442-55d4-4a7c-ba6c-34d278946cf4 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
…ssions 🧩 - OS notifications are kept alive to prevent lost click events 🛡️ - Notification lifecycle cleanup now runs on close and click reliably 🧹 - Agent drawer auto-focuses the search box when opened for faster picking 🎯 - Tab Bar tooltips now reflect user-configured shortcut bindings dynamically ⌨️ - Unread-only session filtering preserves your active session visibility 🧭 - Unread filter now considers unread/busy worktree children, not just parents 🌿 - Worktrees auto-expand while filtering unread so nothing important is hidden 🔎 - Right panel focus ring clears correctly when focus leaves the panel 🎛️ - Focus ring styling no longer bumps z-index, reducing visual layering glitches 🪟
…olish - Tab naming: resolve early from partial output (2s interval) instead of waiting for full process exit; bump timeout 30s → 45s for cold starts - Cue pipeline: prefer subscription name match over stale agent_id when resolving target sessions; fall back to name-based lookup when agent_id is absent - Auto Run: return empty content with notFound flag for missing files instead of throwing, avoiding errors on deleted/renamed documents - LogViewer: add per-entry copy button with clipboard support - AutoRunDocumentSelector: use file icons from theme, move task percentage badge to right-aligned position - Web server: avoid returning stale logs from wrong tab during new tab creation race
Match the tab filter icon pattern where the accent dot is always visible rather than conditional on hasUnreadAgents. Removed the now-unused hasUnreadAgents prop and memo.
…Now for Cue pipelines - Edge-based prompts: each trigger→agent edge can carry its own prompt, enabling multiple triggers to feed the same agent with different instructions - Custom trigger labels: user-defined names (e.g. "Morning Check") displayed on trigger nodes and serialized as subscription labels in YAML - Run Now button: manually trigger any subscription from the Cue dashboard, bypassing event conditions via synthetic event dispatch - Visual polish: pulsing animation on active nodes, larger arrowheads on selected edges, disabled drawer buttons when viewing all pipelines - YAML round-trip: yamlToPipeline deserializes edge prompts and trigger labels; pipelineToYaml generates unique prompt file paths for multi-trigger agents - Tests: 4 new test cases covering edge prompts, custom labels, multi-trigger serialization, and unique prompt file path generation Claude ID: 3ce19e17-50c0-47ff-bc36-f979900ec700 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
Defer splash dismissal until UI has painted with loaded data, preventing the unresponsive gap after the loading screen clears. Progress stages now follow an orchestra warm-up sequence: tuning instruments → reading the score → seating the musicians → warming up the ensemble → the concertmaster rises → maestro takes the podium → curtain up. Claude ID: fc0c8654-1a8f-4a20-a734-fd60d0e73aa3 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
Adds a chevron toggle in the drawer header that expands the node configuration panel from 200px to 80% viewport height. When expanded, textareas switch from fixed rows to flex-fill layout, responsively splitting available space among input/output prompts and per-edge trigger prompts. Includes smooth height transition animation. Claude ID: fc0c8654-1a8f-4a20-a734-fd60d0e73aa3 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
Worktree and wizard sessions now start with only an AI tab. Users can launch terminal tabs on demand. Removes unnecessary createTerminalTab calls and imports from worktreeSession.ts and useWizardHandlers.ts. Claude ID: c57f388f-2e03-48f6-b055-bfb0510fd774 Maestro ID: 373c50f0-14a3-45b7-833f-ccc086211379
The worktree children filter in SessionList was hiding the active session when it had no unread tabs and wasn't busy. Added activeSessionId exemption so the user's current worktree always appears in the left bar.
…pdate tests - Fix DEFAULT_SHORTCUTS.maestroCue → openCue in CueHelpModal (runtime crash) - Remove orphaned auto-scroll toggle JSX and state refs in TerminalOutput - Remove orphaned CueYamlEditor import/state in CueModal (replaced by pipeline editor) - Remove orphaned stats/WakaTime settings and effects in EncoreTab - Remove duplicate filterUnreadAgents branch in useMainKeyboardHandler - Add missing imports in DocumentGenerationView (MermaidRenderer, useClickOutside, etc.) - Prefix unused readOnlyMode param in agent-spawner - Remove unnecessary eslint-disable comment in MainPanel - Update CueModal test: YAML editor assertion → stub no-throw check - Update keyboard handler test: Cmd+T in terminal mode correctly blocked (AI mode only)
SSH Group Chat participants crashed immediately (exit code 1) because large prompts were embedded in bash -c CLI args via buildSshCommand(), where ProcessManager couldn't detect --input-format stream-json flags to enable stdin delivery. Switched to buildSshCommandWithStdin() for large prompts, matching the proven pattern in process:spawn IPC handler. Also added missing SSH wrapping to spawnModeratorSynthesis().
Added a useEffect in App.tsx that tracks inputMode transitions via a ref. When switching from terminal back to AI mode (via tab click, Cmd+J, or any other path), the input textarea is automatically focused so the user can immediately start typing. Claude ID: 3ce1f013-9673-40ec-ac0c-5cf1eaca0198 Maestro ID: 373c50f0-14a3-45b7-833f-ccc086211379
- Session cleanup now kills AI, legacy terminal, and tab PTYs reliably 🔥 - Terminal tabs get proper per-tab process IDs via `getTerminalSessionId` 🧩 - Closing sessions now iterates all `terminalTabs` to terminate each PTY ✅ - Deleting sessions also shuts down every terminal tab process cleanly 🧹 - Improved test coverage for multi-tab PTY termination on close 🧪 - More resilient error handling around terminal-tab process kills 🛡️ - Sentry captures tab-kill failures with session and tab context 🕵️ - Clearer lifecycle comments and intent: “kill all processes” upfront 📝
Cmd+0 was previously consumed by font size reset, blocking the goToLastTab shortcut. Now Cmd+0 goes to the last tab (consistent with Cmd+1-9 tab navigation) and Cmd+Shift+0 resets font size. Font size reset is now customizable in Settings → Shortcuts (moved from FIXED_SHORTCUTS to DEFAULT_SHORTCUTS).
- Splash screen now waits for initial file tree readiness before dismissing 🎬 - Added `initialFileTreeReady` gate to session store initialization flow 🧱 - New splash progress stage highlights file indexing at 80% 🗂️ - UI rendering stage bumped to 90% once file tree finishes loading 📈 - File tree manager signals readiness exactly once on success or error 🔔 - Session restoration unblocks splash immediately when no sessions exist 🧹 - Session-load failures now mark file tree ready to prevent startup hangs 🧯 - App initialization reacts to settings, sessions, and file tree readiness 🔄 - Improved startup messaging: “Indexing the score...” during file discovery 🎻 - Updated tests to cover new three-gate splash behavior and progress ✅
The scroll-into-view logic used full container bounds but didn't subtract the widths of the sticky left (search/filter) and right (+) elements, causing the active tab's close button to be hidden behind them when jumping to a tab or after auto-rename.
Session rows and process rows now show a hover-visible ExternalLink button that navigates to the agent (session) or specific tab and closes the modal. Group chat processes get a similar button that navigates to the group chat.
Show a confirmation modal when closing AI tabs that have draft text or staged images. Applies to single tab close (Cmd+W, click X), and bulk operations (close all, close other, close left, close right).
…S01) Security: filter dangerous flags (--no-sandbox, -y, --dangerously-skip-permissions, etc.) from user-supplied custom args before passing to agent spawn. Logs a warning per stripped arg for auditability. Includes 6 unit tests covering all scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ptimization (TASK-S03) - Add ;, backtick, $ to CMD_SPECIAL_CHARS detection regex - Escape % by doubling to %% in escapeCmdArg (prevents env var expansion in cmd /c) - Add canRunWithoutShell() helper: uses shell:false for resolved .exe paths, avoiding all shell metacharacter risks; documents .cmd limitation for npm-installed CLIs - Apply shell:false optimization in process.ts Windows spawn path - Add 30+ new test cases covering each metacharacter individually for both cmd.exe and PowerShell escaping, plus combined/edge-case scenarios - Add tests for canRunWithoutShell (.exe, .cmd, .bat, basenames, extensionless) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openasocket
force-pushed
the
fix/shared-security-hardening
branch
from
b0cbd15 to
e3795d9
Compare
Contributor
Author
|
Retargeted from |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
applyAgentConfigOverrides()to prevent--no-sandbox,--dangerous-auto-approve,-y, and other security-sensitive flags from being injected viasessionCustomArgs(TASK-S01)escapeArgsForShell()to cover;, backtick,$, and%metacharacters; addcanRunWithoutShell()optimization for resolved .exe paths (TASK-S03)Cherry-picked from
feat/gemini-cliaudit fixes — shared security hardening that applies to all agents.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests