Skip to content

Workspace Artifacts: live, interactive pages in an Artifacts tab - #554

Merged
sambitcreate merged 23 commits into
mainfrom
claude/project-thread-7xf9t3
Oct 19, 2026
Merged

sambitcreate merged 23 commits into
mainfrom
claude/project-thread-7xf9t3

Conversation

@sambitcreate

@sambitcreate sambitcreate commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Requested by sbt · project thread

Before: Aiden could only draw visuals inside a reply. Their data was frozen when they were drawn, and they scrolled away with the chat. Asking for "a page that shows the open PRs" gave a one-off answer.

After: Aiden can build workspace artifacts. These are Aiden-styled pages kept in an Artifacts tab beside the chat, which appears only once the workspace has one. Pages re-read their data from read-only shell commands or workspace files when shown, on Refresh, or on an interval of at least 60 seconds. Their buttons can ask Aiden in the chat, fill the composer, or run a command declared with the page. Every chat in the workspace sees the same pages until someone deletes them from the tab or asks Aiden to.

Access follows the workspace, the same way chats do:

  • No access runs nothing.
  • Under Ask, the approval card shows every command the page can run. Allowing it approves exactly that set, and each button press shows the resolved command first. Workspace files load before approval, and allowing a page reads its sources at once.
  • Full access runs without asking.

This is desktop only (macOS and Linux). It changes nothing in the CLI, on phones, or in the Remote protocol.

How:

  • Shared contract (renderer/shared/workspace-artifacts.ts): parsers and limits (20 pages, 8 sources, 16 actions, 30 s and 256 KiB per source). Command safety:
    • each {{param}} must be a standalone, unquoted argument. It cannot be inside quotes, $(…), <(…), a heredoc or a comment;
    • the parser reads the template as real shell words for each top-level command. It skips assignments, keywords and redirections, joins line continuations, and judges paths, globs and quoted names by what actually runs;
    • a placeholder cannot be the command itself or an argument a command runs as code. That covers runners such as eval, sudo, env and xargs, and an interpreter's code position: sh -c, python -c/-m, node -e, an awk or sed program, sqlite3 SQL and git -c before the subcommand. That list covers common programs, not every program, and the docs say so;
    • a text param cannot sit where bash evaluates names or arithmetic: [[ ]], (( )), let, read, declare or printf -v. Number params can;
    • values are bound to read-only AIDEN_ARG_N shell variables and read as "${AIDEN_ARG_N}", so a template cannot swap the confirmed value for another. Templates may not name those variables;
    • a value cannot start with - or be a negative number, and control or invisible characters are refused.
  • Main (main/services/workspace-artifacts/):
    • a device-local store with snapshots;
    • the workspace_artifact tool (create, update, list, delete), approved per stream by command digest;
    • a runner that shares concurrent refreshes, saves only from the newest run, and re-verifies the folder before anything runs;
    • the built-in aiden-artifacts skill;
    • IPC workspaces:artifacts:{list,rename,delete,saveState,approve,refresh,run}. A button press carries the page digest, and main refuses it if the page changed since.
    • runBoundedShellCommand (shared with run_command) runs nothing when its signal is already aborted.
  • Renderer:
    • an Artifacts Environment tab with a switcher, Refresh, Rename, Delete and "Ask Aiden to change this". Rename and Delete stay tied to the page they were opened on;
    • an approval dialog that notices drift;
    • Run confirmations;
    • page state that persists.
    • AUM gains artifact-only run() and compose(). A page's sendPrompt reaches the chat beside it through its normal send path.
  • Other changes:
    • Removing a workspace removes its artifacts.
    • The skill is hidden on surfaces that never get the tool.
    • There is an onboarding feature-tour tile with its own code-drawn art.
  • Docs: docs/workspace-artifacts.md, README, docs/linux.md, and the plan's "As built" section and index row.

Reviews:

  • Each of the five build phases was reviewed by a separate agent.
  • The review bots' later findings were fixed with tests: a pre-aborted spawn, file sources under Ask, refresh on allow, and confirmed-value binding.
  • A second pass by two reviewers covered code upkeep and adversarial edge cases. Its fixes:
    • command placement and bash arithmetic;
    • parser desyncs from comments, $'…' and backslashes;
    • dialogs tied to their page;
    • shared access rules;
    • a non-tautological IPC channel check through the contract inventory.
  • Later bot rounds hardened the placeholder check: it now reads real shell words, and it refuses interpreter code positions and bash name contexts. Each bypass they found is in the parser tests and the shell oracle.

Tests: npm run test:workspace-artifacts covers:

  • the contract. A real-shell oracle runs every accepted candidate template under /bin/sh, bash --posix and bash, with values that would run code as shell, as a program path, an awk program, a sed script, Python or a git alias;
  • the store, tool, runner and handlers;
  • the panel, with Testing Library;
  • the chat prompt bridge.

The IPC contract test also checks that the artifact handlers in main match the renderer's invokes exactly. The skill registry, AUM block, workspace removal and onboarding suites were extended as well.

CI note: on aa36499, "Android build and APK" failed during Android SDK setup, because the system image download was corrupt (Error on ZipFile unknown archive). Nothing had been built or tested at that point, and the job was re-run once.

Follow-ups:

  • A Playwright end-to-end flow is not in this PR.
  • Releasing the per-stream artifact approval sits beside the seven existing stream exits, not in one shared release step. The subagent exit-sequence contract test pins that order, so moving it is left for a separate cleanup.
  • The feature board picks the feature up after merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
@sambitcreate sambitcreate self-assigned this Oct 19, 2026
Workspace artifacts are render_ui pages a workspace keeps in the Tools
panel. This adds the shared contract, the per-workspace store, the
workspace_artifact tool with its chat approval gate, the IPC handlers,
and run()/compose() actions that compile only for artifacts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
Action values reach the shell only through set -- and "$N", and each
placeholder must be its own unquoted word, so a page can never run what it
passes in. Values may not start with a dash, and commands, labels and values
reject invisible bidi and zero-width characters.

An approval card's command digest is recorded per stream and consumed first
thing by the tool, so a save records an approval only for the commands the
card listed. Invalid stored entries are kept aside instead of lost, a call
that lands after its workspace was removed writes nothing, and the IPC
handlers take their dependencies so a register-and-invoke test covers them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
The tab appears once the workspace has an artifact and closes when the last
one is deleted. A page created by the active chat opens beside it. The tab
draws the selected page with its sources' latest data, switches between
pages, asks the person to allow new commands under Ask, explains when
workspace access is off, and offers Rename, Delete and Ask Aiden to change
this.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
A restored Artifacts tab survives startup and a failed list read, which
now shows an inline retry. The approval dialog freezes the commands it
shows, lists each one separately, approves only that digest, warns when
the page changes underneath, and closes when another page is shown. Page
state survives switching pages. Reveal checks the active workspace, the
error text uses a real token, and the skeleton and status text are
announced properly.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
run_command's bounded shell runner moves to its own module and now also
runs artifact sources (30s, 256 KiB) and buttons (120s) in the workspace
folder. File sources read through the bounded workspace file reader.
Sources follow the workspace's access: None runs nothing, Ask runs
commands only once the person allowed this exact set, Full runs them.
Refreshes of one page share a run, save a snapshot that keeps the last
good data, and notify every window. The Artifacts tab refreshes on open,
on its interval while visible, and from a Refresh button; the agent tool
reports the first refresh to the model.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
- Page buttons run the saved command through workspaces:artifacts:run,
  confirmed under Ask with the exact command shown, and refuse a page whose
  commands changed after the press.
- sendPrompt from a page goes through the visible chat's own send path;
  compose() stages text in its composer.
- Built-in aiden-artifacts skill teaches when and how to build pages.
- Phase 3 review fixes: a shared refresh is keyed by the commands and
  access it runs with, survives one caller cancelling, and never saves
  results for commands that changed; folders are re-verified before
  anything runs; file sources keep the source size limit and a deadline;
  negative number values are rejected; a stale refresh error no longer
  shows on the next page.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
- A refresh started before the page was allowed no longer absorbs the
  next one, and only the newest run for a page saves what it read, so a
  run under older, wider access cannot overwrite a narrower refresh.
- The aiden-artifacts skill is left out of turns, phone catalogs and
  Telegram commands that never get the workspace_artifact tool.
- A button's Run confirmation closes when another page is shown and
  refuses to run once the page changed; a run shows progress, refreshes
  the page that ran, and a second press while one runs is ignored with a
  note.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
- docs/workspace-artifacts.md guide, README and Linux feature lists,
  the plan's "As built" decisions and its index row.
- Onboarding feature tour gains a Workspace Artifacts tile with its own
  code-drawn illustration.
- Removing a workspace, including a managed worktree, removes its
  artifact files.
- Project memory note for the feature's invariants.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
@sambitcreate sambitcreate changed the title docs: plan for workspace Artifacts tab Workspace Artifacts: live, interactive pages in an Artifacts tab Oct 19, 2026
The per-stream artifact approval cleanup now runs before each generation
exit sequence instead of inside it, and workspace removal relies on the
application service alone rather than also removing artifacts in the IPC
handler.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
- The page lists each failed source with its error, so "pulls didn't
  load" comes with the reason.
- A cancelled newer refresh no longer strands the page: the newest live
  run saves, and nothing older than the last save does. The folder is
  resolved before keying, so a linked path shares the run.
- Crash-recovered worktree deletion also removes the workspace's
  artifacts, and failures there are logged.
- Onboarding art rows fit their window; docs no longer call sources
  read-only, and the Remote API notes the desktop-only skill.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
@sambitcreate
sambitcreate marked this pull request as ready for review October 19, 2026 16:55

@very-hermes-bot very-hermes-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline review — 3 finding(s) anchored to the diff. See the pinned summary comment for the overview.

Comment thread main/services/bounded-shell-command.ts
Comment thread renderer/components/artifacts-panel.tsx Outdated
Comment thread renderer/components/artifacts-panel.tsx
@very-hermes-bot

very-hermes-bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Hermes Review Bot

Confidence: 5

Engine: agy/gemini-3.8-flash-high
Review mode: full
Head: aa3649966dca3bacaa4f47b973f317658cf5345a
Generated: 2026-10-19T21:31:54+00:00
Reviews: 1

Summary

This pull request introduces Workspace Artifacts to Aiden Agent: live, interactive, parameterized operational dashboards and action pages stored within workspace directories (.aiden/artifacts/*.json).

Key architectural highlights:

  1. Security & Parameter Binding: Dynamic action parameters in commands (e.g. {{target}}) are strictly validated against injection. They cannot appear inside command names, options, code flags (-c, -e), or arithmetic expansions. During execution, parameters are bound as shell-quoted readonly AIDEN_ARG_N environment variables rather than direct string interpolation, rendering template variable reassignment and parameter pollution impossible.
  2. Approval & Integrity Locks: Under Ask permission modes, every artifact operation (save, update, run, delete) is protected by an SHA-256 digest (workspaceArtifactDigest). Stale approvals or background edits immediately invalidate pending execution digests. Deletion via the agent tool unconditionally mandates user confirmation.
  3. Execution & Concurrency Model: Action execution is isolated through a dedicated runner utilizing generation counters, coalescing in-flight refreshes, ensuring process cleanup upon cancellation or window closure, and validating directory containment against symlink traversals.
  4. Desktop UI & Persistence: A dedicated Artifacts panel in the workspace environment view allows viewing markdown, charts, metrics, and triggering bound actions with pre-run parameter inspection. Storage is backed by atomic writes to .aiden/artifacts/ with automatic directory migration and cleanup on workspace removal.
  5. Cross-Platform Boundaries: Clean separation adheres to platform policy: desktop-only support (macOS & Linux) with no breaking changes or protocol leaks to the Aiden Remote API, CLI, or iOS/Android native clients.

The implementation is accompanied by comprehensive unit, integration, and IPC contract test coverage across all layers.


Confidence Score: 5/5

Inspection confirms exhaustive test coverage (unit test fixtures, shell boundary edge cases, parser stress tests, concurrent runner generation tests, and IPC contracts), robust error-handling, defensive parameter binding, and strict alignment with repo security guidelines and desktop UI conventions.


📁 Important Files Changed
  • renderer/shared/workspace-artifacts.ts: Core data structures, validation rules, AST word tokenization, command safety analysis, SHA-256 digest calculation, and shell parameter binding.
  • main/services/workspace-artifacts/store.ts: File-backed storage layer managing atomic reads and writes in .aiden/artifacts/, snapshot caching, and workspace deletion cascades.
  • main/services/workspace-artifacts/runner.ts: Execution engine coordinating shell execution, signal aborts, generation tracking, and concurrent refresh coalescing.
  • main/services/workspace-artifacts/tool.ts: Agent-facing workspace_artifact tool implementing create, update, delete, and refresh actions with digest enforcement.
  • main/services/bounded-shell-command.ts: Extracted bounded shell execution engine providing consistent timeout, buffer bounding, and process tree termination for coding tools and artifact actions.
  • main/handlers/workspace-artifacts.ts: Secure IPC handlers exposing workspace artifact store operations and execution triggers to the renderer.
  • renderer/components/artifacts-panel.tsx: Interactive frontend surface rendering live cards, form controls, confirmation modals, and execution state.

Findings

No findings.


📊 Sequence Diagram
sequenceDiagram
    autonumber
    actor User
    participant UI as ArtifactsPanel (Renderer)
    participant IPC as WorkspaceArtifactsHandler (Main)
    participant Runner as WorkspaceArtifactRunner
    participant Store as WorkspaceArtifactStore
    participant Shell as BoundedShellCommand

    alt Trigger Action / Refresh
        User->>UI: Click action button / submit parameters
        UI->>UI: Validate parameter rules (no flags, no controls)
        UI->>IPC: invoke("workspace-artifacts:refresh", { id, actionId, params, digest })
        IPC->>Store: getArtifact(workspaceId, artifactId)
        Store-->>IPC: artifact data
        IPC->>IPC: Verify digest matches current artifact state
        IPC->>Runner: refreshArtifact(workspaceDir, artifact, actionId, params)
        Runner->>Runner: Allocate run generation & create AbortController
        Runner->>Runner: Construct script with 'readonly AIDEN_ARG_N=...'
        Runner->>Shell: executeBoundedShellScript(script, cwd, env)
        Shell-->>Runner: stdout, stderr, exitCode
        Runner->>Runner: Verify run generation is still current (not stale)
        Runner->>Store: saveArtifactSnapshot(workspaceId, artifactId, snapshot)
        Store->>Store: Atomic JSON write (.aiden/artifacts/<id>.json)
        Store-->>Runner: saved
        Runner-->>IPC: ArtifactRefreshResult
        IPC-->>UI: ArtifactRefreshResult
        UI->>UI: Render updated output, status badge, and metrics
    end
Loading

Machine-Readable Findings

[]

Last reviewed commit: aa3649966dca
Reviews (1) · Comment /hermes review to trigger a new review · /hermes review full for full re-review

…fresh on allow

A shell command whose signal was already aborted no longer spawns. Under
Ask, a page whose file sources can load refreshes and shows Refresh before
its commands are approved, and allowing the commands reads them at once.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

One command-confirmation mismatch remains: an accepted action template can run against different arguments than the confirmation shows.

Reviewed changes across the complete PR, including the latest cancellation and refresh fixes in e23845f5f.

  • Artifact contract and storage: Adds bounded workspace-owned pages, shell/file sources, parameterized actions, local snapshots and persisted page state.
  • Execution and approval: Adds the desktop agent tool, digest-bound approvals, owner-bound IPC, shared refresh execution and a bounded shell runner reused by run_command.
  • Desktop interactions: Adds the Artifacts tab, page management, per-press command confirmations, composer actions and confirmed chat follow-ups.
  • Lifecycle and distribution: Cleans up artifacts on workspace removal and filters the built-in skill from unsupported generation surfaces, without changing the Remote protocol.
  • Documentation and onboarding: Adds the user guide, implementation plan and themed feature-tour illustration.

Validation: the latest artifact suite passes all 79 tests and TypeScript checking passes. Adjacent compiler, renderer, skill, workspace, shell, onboarding and CI-inventory suites also passed; full Electron E2E was not run. Independent probes confirm the latest cancellation/refresh fixes and reproduce the remaining preview mismatch.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts Outdated
… one that runs

Values were passed as positional parameters, which a template could reset
with set -- or shadow inside a function, so the command that ran could use
a different value from the confirmation. They are now read-only variables.
Also makes the shared-refresh cancellation test wait for both callers to
join before one leaves.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes in 9420a403 since the prior Pullfrog review at e23845f5f, with the full PR diff checked for context.

  • Bound immutable arguments: Replaced mutable positional parameters with quoted, read-only AIDEN_ARG_N variables, addressing the reported confirmation/execution mismatch, and updated the safety documentation.
  • Added shell-oracle coverage: Tested positional mutation, function scope, reassignment, local shadowing and unset attempts against the confirmed value.
  • Adjusted refresh test coordination: Changed caller ordering and timing in the shared-refresh cancellation test without modifying runner behavior.

Validation: all 80 workspace-artifact tests and TypeScript checking passed. Independent /bin/sh and /bin/bash probes confirmed the argument-binding fix. Electron E2E and native macOS execution were not run.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

Adversarial review:
- A {{param}} can no longer be the command itself or an argument that eval,
  sh -c, xargs and similar run, and a text param cannot sit inside [[ ]],
  (( )) or let, where bash evaluates it as arithmetic and can run code.
- The placement parser skips comments, tracks $'…' quoting, refuses
  unbalanced quotes, refuses any placeholder it stepped over (such as one
  after a backslash) and refuses templates naming AIDEN_ARG_ variables.
- Rename and Delete dialogs belong to the page they were opened on, so a
  page deleted elsewhere never leaves the dialog pointing at the next one.

Upkeep:
- One command-access rule shared by shell sources and buttons.
- The runner keeps runSources private, passes approval in, and requires
  the digest for a button press.
- One helper removes a workspace's artifacts; shared title and description
  limits; the approval card text is built from the command list.
- The channel test now checks main's handlers against the renderer's
  invokes through the IPC contract inventory instead of against itself.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new arithmetic check rejects valid action templates even when their text arguments are outside arithmetic contexts.

Reviewed changes in da609fd1 since the prior Pullfrog review at 9420a403, with the complete PR diff checked for context.

  • Hardened command templates: Added command-position and indirect-execution checks, reserved binding names, comment/quote handling, and arithmetic restrictions.
  • Bound management dialogs: Tied Rename and Delete dialogs to the page they were opened on and covered disappearance of that page in a renderer test.
  • Consolidated execution rules: Shared command access, approval descriptions, partial command parsing, limits, and workspace cleanup; made action digests mandatory.
  • Strengthened IPC coverage: Replaced the constant-derived channel assertion with a comparison of actual handler and renderer-invoke inventories.

Validation: all 81 workspace-artifact tests, 22 adjacent IPC/workspace tests, and TypeScript checking passed. Independent shell probes reproduced the inline finding. Electron E2E and native macOS execution were not run.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts Outdated

@very-hermes-bot very-hermes-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline review — 2 finding(s) anchored to the diff. See the pinned summary comment for the overview.

Comment thread renderer/shared/workspace-artifacts.ts Outdated
Comment thread renderer/shared/workspace-artifacts.ts Outdated
The arithmetic check looked at the whole template, so a text placeholder was
refused whenever [[, (( or local appeared anywhere, even in a comment or an
unrelated command. The placement scan now tracks unquoted [[ ]] and (( ))
and the command each placeholder belongs to, and refuses only a text
placeholder inside them or as an argument to let, declare, typeset or local.
It also treats braces and if/then/do-style keywords as command starts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
A placeholder anywhere after eval, sudo, env, xargs and similar in the same
command now counts as theirs (eval echo {{x}} was accepted), and -c only
when the placeholder is the script, so sh -c '…' sh {{x}} still works.
Expansions keep the quoting they opened in, so a ) inside quotes within
$(…) no longer ends the expansion early.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The revised arithmetic check accepts text values that execute shell code on Bash. The expanded executor check also rejects valid action templates.

Reviewed changes in 9b93b152 and d6d88224 since the prior Pullfrog review at da609fd1, with the complete PR diff checked for context.

  • Scoped arithmetic validation: Replaced template-wide rejection with per-placeholder arithmetic-context tracking, fixing the previously reported false-positives.
  • Expanded placement checks: Added command-clause executor checks, more command-start positions, and quote-aware expansion tracking.
  • Extended parser coverage: Added forbidden command/expansion placements and accepted text arguments outside unrelated arithmetic contexts.

Validation: all 81 workspace-artifact tests and TypeScript checking passed. Independent parser/resolver probes with Bash in normal and POSIX modes reproduced both findings below. Native macOS execution and Electron E2E were not run.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts Outdated
Comment thread renderer/shared/workspace-artifacts.ts Outdated

@very-hermes-bot very-hermes-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.

Comment thread renderer/shared/workspace-artifacts.ts Outdated
Find each placeholder's command by skipping reserved words (if, then, do…)
and NAME=value assignments, then decide from that command: nothing before
the placeholder means it is the command; eval, sudo, env, xargs and similar
run it; sh -c runs it only as the script; find -exec runs it; let, declare,
typeset and local read text as arithmetic. Words that only spell a command
name, such as grep's pattern, are data again. Adds a bash oracle: no
accepted template lets a text value run code.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz
The command was found by splitting backwards on ; & | ( ) { }, so a quoted
';' or an expansion like "${HOME}" before the placeholder hid eval. The
forward scan now records where each top-level command starts, unquotes the
command's name ('eval' is eval), and refuses a name chosen at run time.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The revised command classifier newly accepts templates that execute page-supplied values as code. Two security regressions need addressing before merge.

Reviewed changes in 23ce9ba9 and 4fdf4e2a since the prior Pullfrog review at d6d88224, with the complete PR diff checked for context.

  • Identified command names: Replaced clause-wide executor-word matching with command-name checks after reserved words and assignment prefixes, addressing the earlier arithmetic-prefix and incidental-data findings.
  • Tracked top-level clauses: Changed separator handling so quoted literals and expansions do not hide the placeholder's enclosing command.
  • Extended behavioral coverage: Added parser cases for quoted/dynamic executables and ordinary data arguments, plus a real-shell marker oracle.

Validation: all 82 workspace-artifact tests and TypeScript checking passed. Independent parser/resolver probes reproduced both inline findings using real shell/interpreter execution. Native macOS execution and Electron E2E were not run.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts Outdated
Comment thread renderer/shared/workspace-artifacts.ts Outdated
…e interpreter code

The placeholder check now builds each top-level command's words while it
scans, so quoted assignments, redirections (including fds such as 2>&1),
comments and line continuations before a command no longer hide its name.
Absolute paths, globs and braces in the name are judged by what runs.
Interpreters' code positions are refused (sh/bash -c and script, python -c/-m,
node -e, perl/ruby -e, an awk or sed program, sqlite3 SQL, psql -c), and so
are text values where bash reads a variable name (read, printf -v, [ -v).
The shell oracle now also runs each accepted template with a program path,
an awk program, a sed script and Python.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The new redirection handling lets text parameters execute code inside Bash arithmetic. The new Git option check also rejects valid read-only actions.

Reviewed changes in 6084142f since the prior Pullfrog review at 4fdf4e2a, with the complete PR diff checked for context.

  • Rebuilt command scanning: Tracked shell words, redirections, comments and line continuations, addressing the earlier command-prefix bypasses.
  • Expanded code-position checks: Added interpreter-specific option and operand rules, versioned executable handling, runner names and variable-name builtins, addressing the earlier Python finding.
  • Extended behavioral coverage: Expanded the real-shell oracle with multiple payload classes and updated the skill and safety documentation.

Validation: all 82 workspace-artifact tests and TypeScript checking passed. Independent parser/resolver probes reproduced the arithmetic bypass under Bash in normal and POSIX modes; parser and real Git probes confirmed the option-classification regression. Native macOS execution and Electron E2E were not run.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts
Comment thread renderer/shared/workspace-artifacts.ts Outdated
…only before the subcommand

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The new Git option classifier accepts a global configuration value that can execute page-supplied code. One security regression needs addressing before merge.

Reviewed changes in 78df8651 since the prior Pullfrog review at 6084142f, with the complete PR diff checked for context.

  • Preserved arithmetic validation: Distinguished comparisons and shifts inside (( )) and [[ ]] from shell redirections, addressing the previous arithmetic finding.
  • Scoped Git option checks: Added leading-option classification to accept ordinary git grep -c and git log -c arguments, addressing the previous false-positive but introducing the inline finding.
  • Extended behavioral coverage: Added comparison/shift cases to the parser and shell oracle, plus accepted Git subcommand cases.

Validation: all 82 workspace-artifact tests and TypeScript checking passed. Independent parser/resolver probes confirmed the prior fixes and reproduced the new Git code-execution issue with three global-option forms under /bin/sh and Bash POSIX mode. Native macOS execution and Electron E2E were not run.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts Outdated
@pullfrog

pullfrog Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

The model provider returned an error, so this run stopped.

The error is below. If it names your key, plan or balance, fix that with the provider or pick another model before re-triggering.

Model settings → · Ask in Discord →

provider error: This content was flagged for possible cybersecurity risk. If this seems wrong, try rephrasing your request. If you’re doing authorized security work that requires more cyber permissive safeguards, apply for Daybreak access via https://platform.openai.com/settings/organization/status-and-access before retrying.

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using gpt-6.1-sol | 𝕏

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The Git boundary fix remains incomplete: --shallow-file still allows a page-supplied value to execute as a shell alias.

Reviewed changes in 73d25885 since the prior Pullfrog review at 78df8651, with the complete PR diff checked for context.

  • Refined Git command detection: Added separate-valued global options and subcommand-name recognition, blocking the previously reported --git-dir, --work-tree and --namespace examples.
  • Extended behavioral coverage: Added rejected Git-prefix cases and a shell-oracle alias payload in a fresh repository.

Validation: all 82 workspace-artifact tests and npx tsc --noEmit passed. Independent parser/resolver probes confirmed the earlier examples are blocked and reproduced the remaining inline finding under /bin/sh and Bash POSIX mode. Hosted CI is still in progress; native macOS execution and Electron E2E were not run locally.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts Outdated
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The previous Git security finding is fixed, but the new no-value flag list rejects valid actions using --no-literal-pathspecs.

Reviewed changes in 69920fd3 since the prior Pullfrog review at 73d25885, with the complete PR diff checked for context.

  • Hardened Git prefixes: Treated unknown pre-subcommand options as separate-valued unless recognized as no-value flags, closing the reported --shallow-file bypass.
  • Extended behavioral coverage: Added shallow-file and future-option rejection cases, a shell-oracle candidate, and acceptance of a no-pager/separate--C command prefix.

Validation: all 82 workspace-artifact tests and npx tsc --noEmit passed. Independent parser/resolver probes and real execution under /bin/sh and Bash POSIX mode reproduced the inline false-positive; upstream Git source confirms the flag takes no value. Hosted CI remains in progress; native macOS execution and Electron E2E were not run locally.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread renderer/shared/workspace-artifacts.ts Outdated
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJWk6Vt57gfZkp14JWPXSz

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes in aa364996 since the prior Pullfrog review at 69920fd3, with the complete PR diff checked for context.

  • Corrected Git flag recognition: Recognized --no-literal-pathspecs as a no-value global flag, fixing the previously reported rejection of valid action templates.
  • Extended behavioral coverage: Added an accepted git --no-literal-pathspecs log -c {{x}} case while retaining the sensitive global-option rejection cases.

Validation: all 82 workspace-artifact tests and npx tsc --noEmit passed. Independent parser/resolver probes successfully executed log/grep actions with five supported pathspec flags under /bin/sh and Bash POSIX mode, and confirmed that global -c placeholders remain rejected with plain, git-dir, shallow-file and unknown-option prefixes. Upstream Git option handling also confirms the corrected flag consumes no value. Native macOS execution and Electron E2E were not run.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

Copy link
Copy Markdown
Owner Author

Android build and APK failed on aa36499 during Android SDK setup, before anything was built or tested. The download of the Google APIs x86_64 system image (x86_64-36_r07.zip) came back corrupt (Error on ZipFile unknown archive). This PR doesn't change the Android app or its CI setup.

GitHub won't re-run the job while the rest of the workflow is still going, so I'll re-run it once, as soon as the workflow finishes.


Generated by Claude Code

@sambitcreate
sambitcreate merged commit 0a73c0e into main Oct 19, 2026
49 of 51 checks passed
@sambitcreate
sambitcreate deleted the claude/project-thread-7xf9t3 branch October 19, 2026 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants