release: promote development onto main - #101
Conversation
The lws thread still calls auth_validate_token after SIGTERM. Freeing auth_ctx first raced with in-flight /api and WebSocket auth checks. Co-authored-by: esadrianno <esadrianno@gmail.com>
Default workspace_path was ~/.shellclaw, the same tree as pairing tokens, memory.db, and config.toml. With workspace_only on, inbound Discord/webchat/cron file tools could read or overwrite those files. Point the default at ~/.shellclaw/workspace and deny runtime state paths even when an operator keeps the old workspace root. Co-authored-by: esadrianno <esadrianno@gmail.com>
fix(dispatch): acquire agent mutex around main-loop agent_run
Inbound mcp.tool_call dispatched tools without the global agent mutex while task.request already held it, allowing concurrent session/memory access from HTTP threads during agent_run. Refs: #60
state.query read SQLite g_db from HTTP threads without agent_lock, the same cross-thread contract as mcp.tool_call. Lock around hook and row-count reads, unlock before envelope JSON, and probe both paths in tests. Refs: #60
Thread-safety notes still described the mutex as agent_run-only. Align README, ARCHITECTURE, and CONTRIBUTING with the inbound ASAP callers. Refs: #60
fix(asap): acquire agent mutex around inbound mcp.tool_call dispatch
Inbound task.request used the hardcoded session "asap:inbound", leaking conversation history across unrelated clients. Derive per-sender session ids from the envelope sender URN. Refs: #64
snprintf into the session-id buffer would clip long senders and reintroduce history collisions. Match discord session helpers: reject instead of truncating. Always derive the id from the envelope sender so a ctx override cannot restore a shared bucket. Refs: #64
POST /asap is protocol-public; empty trusted_senders allows every claimed URN. Production must set the allowlist so asap:<sender> sessions cannot be spoofed. Refs: #64
…ent-32b5 fix(asap): isolate inbound task.request sessions by sender URN
handle_asap zeroed asap_server_ctx_t and only set cfg, so inbound task.request always failed with "server missing cfg or provider" and mcp.tool_call saw an empty tool table. Bind the bootstrap provider and flattened tools the same way handle_message does. Refs: #53
handle_message and handle_asap both flattened the tool table by hand. One helper keeps the filled count honest on a NULL slot. Refs: #53
Parse the JSON-RPC result instead of strstr, print the body on HTTP mismatch, and cover mcp.tool_call of an unknown name (-32001). Refs: #53
POST /asap is rate-limited, not Bearer. Empty trusted_senders allows any URN to reach agent_run and direct mcp.tool_call. Production must set the allowlist before exposing the gateway. Refs: #53
…ent-412d fix(asap): wire provider and tools into POST /asap
Inbound POST /asap copied serialized JSON-RPC into the 64 KiB gateway buffer with silent truncation, which produced invalid JSON. Reject with JSON-RPC -32603 when the payload cannot fit. Refs: #61
asap_log_append_out ran before write_asap_jsonrpc, so /api/asap/log showed mcp.tool_result that the caller never received. Refs: #61
…ent-6a31 fix(gateway): reject oversized ASAP responses instead of truncating JSON
…-5561 fix(asap): avoid double-free on malformed JSON-RPC results
Publish text_cap/tool_cap only after realloc succeeds. On grow failure, free parse scratch and return an error instead of memcpy/index against an inflated cap. Refs: #80
…-3c61 fix(providers): fail closed when Anthropic parse realloc fails
Skip remove() when sqlite3_open fails on a path that already exists. Deleting it caused silent data loss on permissions or transient I/O. Refs: #63
fix(gateway,skills): persist pairing tokens and skills with atomic replace
Assign payload directly to out->payload. The extra cJSON_Delete is already gone; the local alias was only for cppcheck.
Fail closed if a later caller forgets to pass &conn->body instead of skipping the 64 KiB check.
Keep the leftover 64 KiB static BODY_BUF_SIZE check off inbound /asap.
Seven counted POSTs in the 60s window leave 3 of 10 slots; the 1 MiB 413 path is LWS init and is not counted.
fix(asap,gateway): inbound envelope OOM free and 1 MiB POST /asap cap
Formatter-indented keys and commented section headers were missed, so a dashboard save inserted a duplicate key and tomlc99 returned 400. A present field with the wrong JSON type now fails instead of a 200 no-op, and string values escape quotes and newlines.
Dashboard save and SIGHUP both swap the live config. Hold agent_lock on both paths so the same pointer cannot be queued twice, and return HTTP 500 when the file is saved but the live reload does not.
fix(gateway): apply dashboard JSON config and reload live settings
Keep the default workspace off ~/.shellclaw and the runtime-state denylist together with development's atomic writes, symlink rejection, and Jetson allowlist entries.
GCC -Werror=format-truncation rejected snprintf of a 256-byte directory plus /config.toml into another 256-byte buffer.
WAL sidecars next to memory.db were readable, and a bare cat config.toml ran after the sandbox chdir. Also refuse a symlink workspace and apply the same state-file check on the unsandboxed shell path.
…-82df fix(sandbox): keep agent workspace off gateway state files
workspace_only tokenized on whitespace, so a quoted path, a file: URL, or $HOME/$PWD could leave the workspace, and strdup failure fell open. Record the unresolved-path reason before freeing the command copy. Substring-block auth_tokens.json, shellclaw.pid, and shellclaw.log on the whole command so python -c open() cannot skip the basename check.
unshare does not move the caller into the new PID namespace, and a shared mount tree would let a later umount of /proc hit the host. Fork so the command is PID 1, require MS_PRIVATE, remount proc, and report setup failure on a control pipe: the child writes the byte, and EOF is success. Join the cgroup before that fork, reap on timeout, and close inherited fds.
The string allowlist cannot see interpreter concatenation such as chr(47)+. Enter the workspace before restrict_self, grant read on /etc/ssl/certs only, and split prepare() from restrict_self so gcov can still write. Landlock setup failure is fail-closed on the isolation pipe.
Namespace setup is fail-closed, the mount tree is made private, and proc is remounted for PID 1. Landlock, not the string allowlist, is the kernel host-FS bound when a workspace path is set.
…ndlock fix(sandbox): Landlock FS bound plus workspace_only defense-in-depth
Deferred ack kept the earliest due row at the head of the queue, so a failed send or agent run blocked every later reminder until that job succeeded.
A full ring used to drop the oldest id, and the next poll treated that job as cold, so a down channel tight-looped agent_run. Reuse a slot only after its recorded window has elapsed, and wait on the earliest due job when every slot is still hot.
…ation fix(cron): deliver other due jobs while one remains unacked
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
ShellClaw is a C99 edge agent (Jetson / Raspberry Pi): a single-threaded ReAct loop, libwebsockets gateway, and a Linux namespace plus Landlock shell sandbox. This PR promotes development onto main after the Bugbot batches: fail-closed sandbox and Landlock, cron delivery that stays due until ack, dashboard JSON config patch plus live reload, ASAP body cap and agent_lock, and atomic file/auth/skill writes.
The earlier blockers I tracked on those batches are fixed on this tip (PID-1 timeout with PR_SET_PDEATHSIG and a /proc scan, tracked-id re-offer wait, slash-command cron ack, mkstemp writes, unsandboxed process-group kill). I do not see a merge-blocking bug. static, test, release on this PR was still pending when I looked; the previous development run passed. Wait for this PR's run before merging, and merge without squash as the body says.
Must Fix
None.
Should Fix
-
src/tools/cron.coffer table (CRON_OFFER_TRACK16). A due id that is not already in the table is skipped while every slot is still insidetimeout_ms(cron_due_is_returnable). The fallback always re-fetcheskeys[0]andcron_wait_remainingreturns immediately when that id was never stored. With 17 due jobs,j16is not offered whilej00–j15stay due;test_cron_poll_waits_when_offer_table_is_fulllocks that. After those 16 are acked, their slots stay hot, so the next untracked due job is returned with no wait andcron_mark_offereddrops the mark. A failedagent_runthen busy-spins until a slot ages out. The changelog line that says a stuck job cannot hide later jobs is only true for two jobs. -
src/sandbox/sandbox_landlock.cgrants RW/dev/zero,/dev/urandom, and/dev/tty, and addsLANDLOCK_ACCESS_FS_IOCTL_DEVto that file mask.docs/SECURITY.mdstill says the ruleset is RW/dev/nullonly.setsid()in the isolator means/dev/ttyopen should fail withENXIO, so this is least privilege and doc drift, not a live TIOCSTI path.landlock_add_pathalso ignoreslandlock_add_rulefailure ((void)rc). -
Config save did not pick up the atomic replace used for files, skills, and pairing tokens.
handle_config_putwritesconfig.toml.tmpwithfopen("w"), andvalidate_patched_tomlwritesconfig.toml.patch-testthe same way. Both follow a symlink and can truncate the target. Neitherfsyncs beforerename. -
handle_config_putcallshttp_set_live_config(bootstrap_get_cfg())afteragent_unlock.try_config_reloadalready publishes the pointer under the lock. A SIGHUP reload in between can leave the gateway holding the previousconfig_tuntil the next reload (not a use-after-free;stale_free_allruns at shutdown).
Nice to Have
LWS_CALLBACK_RECEIVEinsrc/gateway/http_lws.cstillcJSON_Parses each callback and never checkslws_is_final_fragment.WS_RX_BUFFER_SIZEcovers a normal 32 KiB chat frame. A larger or deliberately split frame is still dropped.src/sandbox/allowlist.cis about 780 lines after the path split. Further growth belongs in another file, not more scanner cases.- Add a cron test that acks 16 jobs inside the timeout window and asserts the 17th id is both delivered and subject to the re-offer wait if delivery fails.
Positive Highlights
- Sandbox isolation fails closed on a control pipe, the command is PID 1, proc is remounted, and the timeout test scans
/procfor a leftoversleepinstead of trusting the output string. - Pairing tokens, skills, and workspace writes use unique
mkstempplusfsyncandrename, with regression tests for the symlink and truncation cases those bugs had. - Dashboard JSON patching rejects a wrong JSON type, updates indented keys, and takes
agent_lockaround reload so it cannot race the SIGHUP path on the same pointer.
Sent by Cursor Automation: Code review
A full offer table returned the next id immediately because that id had no stored age. Sleep the poll timeout in that case, and free the slot when the job is acked so the following due job can be delivered and then backed off.
Device nodes stay writable without IOCTL_DEV, and a failed landlock_add_rule now rejects the ruleset instead of continuing with a partial grant. The security note lists the same device paths.
PUT /api/config and the JSON patch check used predictable sidecars that follow a symlink. Write those temps with mkstemp and fsync, and leave the live gateway config pointer to the reload that already runs under the agent lock.
test_reload rebuilds reload.o without the gateway, so the publish inside try_config_reload is not in the shellclaw binary. Set the gateway pointer while the lock is still held; a later call could put the previous config back.
2 participants
Summary
Promotes the integration branch onto
mainafter the Bugbot rewrite batches landed ondevelopment(including #89 sandbox/Landlock and #100 cron sibling delivery).workspace_onlyallowlist fails closed (quoted paths,file:URLs,$HOME/$PWD, reserved runtime names). Landlock is the kernel host-FS bound. Namespace setup fails closed, the command is PID 1, and cgroup limits attach before the command fork.timeout_ms, and a stuck job no longer hides later due jobs.PUT /api/configpatches TOML and reloads live settings;POST /asaphonors the 1 MiB body cap and dispatches underagent_lock; listen bind followsgateway.host.Test plan
static, test, releasegreen ondevelopmentafter fix(sandbox): Landlock FS bound plus workspace_only defense-in-depth #89 (337fb31) and fix(cron): deliver other due jobs while one remains unacked #100 (aa6cd98)mainas base) is green