Skip to content

fix: audit batch — 10 verified bugs (security, plugin, skill, UI) - #111

Merged
yogthos merged 1 commit into
mainfrom
fix/audit-batch-1-critical
May 21, 2026
Merged

fix: audit batch — 10 verified bugs (security, plugin, skill, UI)#111
yogthos merged 1 commit into
mainfrom
fix/audit-batch-1-critical

Conversation

@yogthos

@yogthos yogthos commented May 21, 2026

Copy link
Copy Markdown
Collaborator

10 verified-real bugs from the 23-item audit batch (8 false-positives confirmed; docs/architecture fixes in a follow-up PR). Includes: bash pipe-split security, read.rs binary detection (ported from opencode), skill override correctness, broken plugin hooks, MCP malformed JSON, /prompt default, /allow add validation, grep file-size cap, Python dunder methods, panel Unicode width. 4 new tests, 725 plugin / 599 default pass.

…aths

23 audit findings verified REAL via parallel agent verification +
cross-check against opencode/pi reference patterns. Shipping the
10 most concrete fixes here; the rest go in a follow-up docs/test
batch.

## Security

- **#9 bash quote_aware_split missed bare `|`** —
  `safe_cmd | rm -rf /` was treated as one segment; only the
  LHS got permission-checked. Pipe RHS rode in unchecked under
  the fallback (non-semantic-bash) path. Added single-byte `|`
  split after `||` is matched. The tree-sitter path was already
  correct.

- **#4 read.rs no binary detection** — feeding a PDF/ELF/.pyc
  into the LLM as lossy UTF-8 wasted tokens and confused the
  model. Ported opencode `read.ts:153-198`: reject by
  extension list (zip/exe/.o/.pdf/.png/etc.), then sniff the
  first 4 KiB — null byte = binary, >30% non-printable = binary.
  Clear error message tells the agent to use bash + xxd instead.

## Correctness

- **#2 skill override inverted** — README contract: "Project
  skills override global skills by name". Code used
  `map.entry(name).or_insert(skill)` which KEEPS the first
  (global) value and silently drops project overrides. Switch
  to `map.insert` (last-write-wins) since globals iterate
  first and project iterates second.

- **#37 skill empty name** — frontmatter `name:` with empty
  value parsed to "", which then matched any `skill ""` call
  silently. Fall back to directory name when frontmatter name
  is empty/whitespace-only.

- **#1 session_tree.janet hook never fired** — plugin defined
  `(defn on-message ...)` but `(def hooks [])` was empty AND
  the hook name doesn't exist (dirge uses `on-message-update`).
  `/label` was permanently broken ("no entry yet"). Fix:
  rename to `on-message-update` + register in hooks vector.

- **#7 workflow.janet hooks vector missing entries** — plugin
  defined `workflow-on-tool-end`, `-on-error`, `-on-complete`
  but only registered the first four hook names. Three hooks
  were dead. Added them.

- **#26 MCP malformed JSON silently empty args** —
  `serde_json::from_str(&args).unwrap_or_default()` turned bad
  JSON into None, sending the server an empty argument set.
  Server then errored with confusing "missing required field"
  instead of dirge surfacing the actual parse error. Now returns
  ToolError with the parse error message + first 200 chars of
  the offending JSON.

- **#22 /prompt default unreachable** — README documents
  `default` as a built-in prompt (prompts/default.md exists),
  but `/prompt default` was intercepted as a magic "clear"
  keyword. If `default` is registered in `context.prompts`,
  the new branch falls through to the normal name-lookup. Only
  acts as clear-keyword when no `default` prompt is present
  (legacy fallback).

- **#23 /allow add accepted invalid tools** — typo
  `/allow add bsah ...` silently created an inert rule the
  user couldn't debug. Added a known-tools whitelist matching
  PermissionConfig fields; unknown tools error with the valid
  list.

## Performance + correctness

- **#11 grep loaded whole files into memory** — no size cap
  meant a 9MB file got fully buffered. Added 10 MiB per-file
  cap via metadata pre-check.

- **#15 Python dunder methods marked non-exported** —
  `!name.starts_with('_')` treats `__init__`/`__call__`/etc.
  as private, even though they're Python's standard public
  protocol. Recognize `__x__` dunder pattern as exported.

## UI

- **#36 panel char-count truncation vs Unicode width** — panel
  truncation used `chars().count()` while wide emoji and CJK
  take 2 cells. A status line with an emoji overflowed the
  right border by one cell. Switched to
  `UnicodeWidthStr::width` for both truncation and padding.

## Tests

4 new regression tests:
- `test_is_binary_extension_known` — pdf/tgz/.so/.jpg/.pyc
- `test_is_binary_content_null_byte` — null byte trigger,
  UTF-8 Japanese stays clean, all-non-printable triggers
- `quote_aware_split_splits_on_bare_pipe` — pipe security
- `quote_aware_split_or_and_pipe_distinct` — `a || b | c`
  produces 3 segments, not 2

725 plugin / 599 default pass. All build profiles clean.

## Verified false positives (not fixed, audit was wrong)

- #3 cache.rs clear() race — generation counter gating in
  `get` makes stale entries invisible, no correctness impact.
- #17 DeepSeek auto-detect priority — auto-detect only fires
  when env vars present; default-default is still OpenRouter.
- #19 semantic tools in collision filter — semantic tools
  added separately, can't be shadowed by MCP.
- #20 glob global gitignore — intentionally disabled to match
  grep behavior.
- #28 nearest_root blocking std::fs — function doesn't exist
  in current code.
- #32 ReadArgs.path vs GrepArgs.path — semantically different
  by design (file vs dir), documented in schema.
- #33 install_plugin_providers dead-without-feature — gated
  with explicit `#[cfg_attr(not(feature), allow(dead_code))]`.
- #34 websearch double-gated — config + API key serve distinct
  purposes (enable + auth).

## Deferred to follow-up batches

Docs-only fixes (#6 CONFIG.md tools, #12 temperature, #13
--api-key, #14 acp_host/port), MCP/LSP architecture (#8, #25,
#27), test gaps (#38-40), and lower-priority polish — all in
a follow-up PR.
@yogthos
yogthos merged commit 742e875 into main May 21, 2026
1 check passed
@yogthos
yogthos deleted the fix/audit-batch-1-critical branch May 21, 2026 17:40
yogthos added a commit that referenced this pull request May 21, 2026
Follow-up to PR #111. Tier-2 items from the 23-bug audit batch:
docs corrections and two small correctness/UX fixes.

## Docs

- **#12 temperature** — CONFIG.md claimed "parsed but not
  currently applied". Actually applied since PR #105 with a
  clamp warning. Rewrote the cell.
- **#13 --api-key** — flag existed but neither README nor
  CONFIG.md mentioned it. Added a Quick-start example noting
  the process-list visibility caveat.
- **#14 acp_host/acp_port** — CONFIG.md documented both keys
  but the CLI flags were intentionally removed (stdio-only
  transport). Removed both from the keys table + ACP section.
- **#6 tools** — `Config::tools` (per-tool enable map) was
  fully wired in code but undocumented. Added a row to the
  keys table covering `tools.websearch` and `tools.webfetch`.
- **#21 find_callers** — README claimed "word-boundary regex"
  but the impl uses the tree-sitter symbol index. Updated to
  reflect actual behavior; the user-visible word-boundary
  semantics are preserved.

## Code

- **#16 semantic index skip_dir** — `SymbolIndex::find_callers`
  filter had its own hardcoded `matches!(name, "node_modules"
  | "target" | ".git" | "__pycache__")` while the rest of
  the codebase uses `agent::tools::is_skip_dir`. Switched to
  the shared helper so future additions stay in lockstep.

- **#18 context::load_file** — silently swallowed
  `read_to_string` errors via `.ok()`. A permission-denied
  AGENTS.md looked identical to a missing file. Now emits a
  stderr warning naming the path + reason; still returns None
  so callers' behavior is unchanged.

725 plugin / 599 default pass. All build profiles clean.

## Remaining audit items (deferred to feature work)

- **#8 LSP no crash restart**: needs broken-pipe IO error
  handling + exponential backoff. Touches manager state
  machine.
- **#10 task tool fire-and-forget**: needs timeout + cleanup
  coordination via JoinHandle tracking.
- **#25 MCP no reconnection**: similar architectural concern
  to #8.
- **#27 LSP didClose**: client lifecycle hook missing.
- **#29 token estimation len/4**: needs per-provider usage
  extraction (Phase 6 work).
- **#5 MCP shutdown**: rmcp Drop semantics need verification.
- **#38/39/40 semantic test gaps**: get_symbol_body untested,
  list_symbols kind_filter untested, find_definition test
  vacuous. Sat down to add but each requires a fixture build.

Together with PR #111 (10 code fixes), 17 of the 23 verified
items are now shipped. Remaining 6 are architectural or
test-infrastructure work better tackled as discrete PRs.

Co-authored-by: Yogthos <yogthos@gmail.com>
yogthos added a commit that referenced this pull request May 21, 2026
…1 leftovers (#113)

Verified the 11 new round-2 findings and 6 round-1 leftovers
against the actual code. Shipping the 7 concrete fixes that map
to clear before/after behavior; the rest are deferred-architectural
or already-fixed-elsewhere.

## N1 — webfetch SSRF (CRITICAL)

`http://169.254.169.254/latest/meta-data/` (AWS metadata) and
all private/loopback ranges sailed straight through. An LLM that
gets prompt-injected into fetching a private IP could exfiltrate
IAM credentials, hit internal admin endpoints, or scrape internal
network maps.

New `validate_url_host_safety()` blocks:
- Hostname literal "localhost" / "ip6-localhost" /
  "ip6-loopback"
- IPv4 loopback (127/8), private (10/8, 172.16/12, 192.168/16),
  link-local (169.254/16 — cloud metadata), unspecified,
  broadcast, class E.
- IPv6 loopback, ULA (fc00::/7), link-local (fe80::/10),
  multicast, unspecified. Bracketed IPv6 (`[::1]`) is parsed
  correctly.

Opt-in escape hatch: `DIRGE_WEBFETCH_ALLOW_PRIVATE=1` for
dev workflows that need to hit localhost. Tool description
documents the env var so the agent can see it.

## N8 — webfetch body OOM (HIGH)

`resp.text().await` buffered the entire response into memory
before any truncation. A 500 MB page would OOM the agent
process. Now streams via `bytes_stream` with a 10 MiB cap;
bails at the cap.

## N2 — MCP result size cap (CRITICAL)

`McpTool::call` concatenated every `RawContent::Text` block
into `content` without limit. A 200 KB MCP response flooded
every subsequent turn until compaction. Cap aggregate at 256
KiB (≈65k tokens worst case); UTF-8 char-boundary slice;
append a clear truncation marker naming the offending
server::tool so the agent can adjust its calls.

## N3 — bash output cap (CRITICAL)

The UI's `render_tool_output` truncated the DISPLAY but the
full string was persisted to `ToolCallState::Completed` and
fed back to the LLM on the next turn. `cat /dev/urandom | head
-c 10M` would have shoved millions of tokens. Cap at 256 KiB
at the bash tool source, with a clear truncation message
pointing the agent at head/grep filtering.

## N5 — sandbox hardening (HIGH)

Added `--new-session` (drops the ability to gain new privs via
setuid) and `--unshare-user-try` (explicit user-ns isolation,
defensive against future bwrap default changes). `--unshare-all`
already covers most of this; the explicit flags pin the
guarantees.

## N7 — sanitize_output strips \\r (HIGH)

Previously `\\r` was preserved (originally for CRLF lines). A
bash tool printing progress with `\\rstep N/M` would move the
cursor to column 0 mid-line, overwriting chamber borders. Now
stripped (`\\n` and `\\t` still pass; display path
normalizes CRLF before reaching here).

## H10 — task subagent timeout

`tokio::spawn(btw_query(...))` with no timeout meant a stuck
provider would keep the task in `Running` forever. Cap at 10
minutes; on timeout, the task transitions to `Failed` with a
clear message and the system-reminder still fires.

## M22 — /prompt default help text

Behavior was fixed in PR #111 (activates `default.md` if
installed). Help text still said "clear active prompt". Now
explains both cases.

## Verified false positives / non-issues this round

- **N4 sandbox --ro-bind / /**: by design — the sandbox is a
  WRITE guard, not exfiltration prevention. Defense for read
  access requires `--ro-bind` per path (much narrower scope
  + breaks read-only tooling like bash `ls`). Document as
  feature, not bug.
- **N6 sandbox current_dir fallback**: `unwrap_or_else(|_|
  ".".into())` is the standard fallback; `current_dir()`
  failing means cwd is deleted/inaccessible which is its own
  fatal-soon problem.
- **N9 token estimation len/4**: Phase 6 work; per-provider
  usage extraction needs distinct fix.
- **N10 sandbox /dev/tty**: `--dev /dev` mounts a tmpfs with
  only essential nodes, not the full host /dev. /dev/tty IS
  available to the sandboxed process so it can write to its
  own controlling terminal (needed for bash interactive
  features); not the host's /dev/tty0.
- **H17 DeepSeek auto-detect order**: confirmed false positive
  in earlier rounds — only fires when env var present.
- **H18 load_file path in error**: already fixed in PR #112.
- **M19 semantic tools in BUILTIN_TOOL_NAMES**: semantic tools
  added separately, can't be shadowed by MCP collision.

## Tests

2 new regression tests:
- `validate_url_host_safety_blocks_ssrf_targets`: pins
  169.254.x, 127.x, RFC 1918, ::1, fc00::, fe80:: as refused;
  public IPs + domains pass; bracketed IPv6 parses correctly.
- `validate_url_host_safety_handles_malformed_hosts`: garbage
  hosts don't panic.

727 plugin / 601 default pass.

## Deferred to architectural follow-ups

- H8 LSP no crash restart — broken-pipe handling design
- N4 sandbox --ro-bind / — fundamental design tradeoff
- N9 token estimation — per-provider usage extraction

Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch pushed a commit to allen-munsch/dirge that referenced this pull request Jun 3, 2026
…aths (dirge-code#111)

23 audit findings verified REAL via parallel agent verification +
cross-check against opencode/pi reference patterns. Shipping the
10 most concrete fixes here; the rest go in a follow-up docs/test
batch.

## Security

- **dirge-code#9 bash quote_aware_split missed bare `|`** —
  `safe_cmd | rm -rf /` was treated as one segment; only the
  LHS got permission-checked. Pipe RHS rode in unchecked under
  the fallback (non-semantic-bash) path. Added single-byte `|`
  split after `||` is matched. The tree-sitter path was already
  correct.

- **#4 read.rs no binary detection** — feeding a PDF/ELF/.pyc
  into the LLM as lossy UTF-8 wasted tokens and confused the
  model. Ported opencode `read.ts:153-198`: reject by
  extension list (zip/exe/.o/.pdf/.png/etc.), then sniff the
  first 4 KiB — null byte = binary, >30% non-printable = binary.
  Clear error message tells the agent to use bash + xxd instead.

## Correctness

- **#2 skill override inverted** — README contract: "Project
  skills override global skills by name". Code used
  `map.entry(name).or_insert(skill)` which KEEPS the first
  (global) value and silently drops project overrides. Switch
  to `map.insert` (last-write-wins) since globals iterate
  first and project iterates second.

- **dirge-code#37 skill empty name** — frontmatter `name:` with empty
  value parsed to "", which then matched any `skill ""` call
  silently. Fall back to directory name when frontmatter name
  is empty/whitespace-only.

- **#1 session_tree.janet hook never fired** — plugin defined
  `(defn on-message ...)` but `(def hooks [])` was empty AND
  the hook name doesn't exist (dirge uses `on-message-update`).
  `/label` was permanently broken ("no entry yet"). Fix:
  rename to `on-message-update` + register in hooks vector.

- **dirge-code#7 workflow.janet hooks vector missing entries** — plugin
  defined `workflow-on-tool-end`, `-on-error`, `-on-complete`
  but only registered the first four hook names. Three hooks
  were dead. Added them.

- **dirge-code#26 MCP malformed JSON silently empty args** —
  `serde_json::from_str(&args).unwrap_or_default()` turned bad
  JSON into None, sending the server an empty argument set.
  Server then errored with confusing "missing required field"
  instead of dirge surfacing the actual parse error. Now returns
  ToolError with the parse error message + first 200 chars of
  the offending JSON.

- **dirge-code#22 /prompt default unreachable** — README documents
  `default` as a built-in prompt (prompts/default.md exists),
  but `/prompt default` was intercepted as a magic "clear"
  keyword. If `default` is registered in `context.prompts`,
  the new branch falls through to the normal name-lookup. Only
  acts as clear-keyword when no `default` prompt is present
  (legacy fallback).

- **dirge-code#23 /allow add accepted invalid tools** — typo
  `/allow add bsah ...` silently created an inert rule the
  user couldn't debug. Added a known-tools whitelist matching
  PermissionConfig fields; unknown tools error with the valid
  list.

## Performance + correctness

- **dirge-code#11 grep loaded whole files into memory** — no size cap
  meant a 9MB file got fully buffered. Added 10 MiB per-file
  cap via metadata pre-check.

- **dirge-code#15 Python dunder methods marked non-exported** —
  `!name.starts_with('_')` treats `__init__`/`__call__`/etc.
  as private, even though they're Python's standard public
  protocol. Recognize `__x__` dunder pattern as exported.

## UI

- **dirge-code#36 panel char-count truncation vs Unicode width** — panel
  truncation used `chars().count()` while wide emoji and CJK
  take 2 cells. A status line with an emoji overflowed the
  right border by one cell. Switched to
  `UnicodeWidthStr::width` for both truncation and padding.

## Tests

4 new regression tests:
- `test_is_binary_extension_known` — pdf/tgz/.so/.jpg/.pyc
- `test_is_binary_content_null_byte` — null byte trigger,
  UTF-8 Japanese stays clean, all-non-printable triggers
- `quote_aware_split_splits_on_bare_pipe` — pipe security
- `quote_aware_split_or_and_pipe_distinct` — `a || b | c`
  produces 3 segments, not 2

725 plugin / 599 default pass. All build profiles clean.

## Verified false positives (not fixed, audit was wrong)

- #3 cache.rs clear() race — generation counter gating in
  `get` makes stale entries invisible, no correctness impact.
- dirge-code#17 DeepSeek auto-detect priority — auto-detect only fires
  when env vars present; default-default is still OpenRouter.
- dirge-code#19 semantic tools in collision filter — semantic tools
  added separately, can't be shadowed by MCP.
- dirge-code#20 glob global gitignore — intentionally disabled to match
  grep behavior.
- dirge-code#28 nearest_root blocking std::fs — function doesn't exist
  in current code.
- dirge-code#32 ReadArgs.path vs GrepArgs.path — semantically different
  by design (file vs dir), documented in schema.
- dirge-code#33 install_plugin_providers dead-without-feature — gated
  with explicit `#[cfg_attr(not(feature), allow(dead_code))]`.
- dirge-code#34 websearch double-gated — config + API key serve distinct
  purposes (enable + auth).

## Deferred to follow-up batches

Docs-only fixes (dirge-code#6 CONFIG.md tools, dirge-code#12 temperature, dirge-code#13
--api-key, dirge-code#14 acp_host/port), MCP/LSP architecture (dirge-code#8, dirge-code#25,
dirge-code#27), test gaps (dirge-code#38-40), and lower-priority polish — all in
a follow-up PR.

Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch pushed a commit to allen-munsch/dirge that referenced this pull request Jun 3, 2026
…de#112)

Follow-up to PR dirge-code#111. Tier-2 items from the 23-bug audit batch:
docs corrections and two small correctness/UX fixes.

## Docs

- **dirge-code#12 temperature** — CONFIG.md claimed "parsed but not
  currently applied". Actually applied since PR dirge-code#105 with a
  clamp warning. Rewrote the cell.
- **dirge-code#13 --api-key** — flag existed but neither README nor
  CONFIG.md mentioned it. Added a Quick-start example noting
  the process-list visibility caveat.
- **dirge-code#14 acp_host/acp_port** — CONFIG.md documented both keys
  but the CLI flags were intentionally removed (stdio-only
  transport). Removed both from the keys table + ACP section.
- **dirge-code#6 tools** — `Config::tools` (per-tool enable map) was
  fully wired in code but undocumented. Added a row to the
  keys table covering `tools.websearch` and `tools.webfetch`.
- **dirge-code#21 find_callers** — README claimed "word-boundary regex"
  but the impl uses the tree-sitter symbol index. Updated to
  reflect actual behavior; the user-visible word-boundary
  semantics are preserved.

## Code

- **dirge-code#16 semantic index skip_dir** — `SymbolIndex::find_callers`
  filter had its own hardcoded `matches!(name, "node_modules"
  | "target" | ".git" | "__pycache__")` while the rest of
  the codebase uses `agent::tools::is_skip_dir`. Switched to
  the shared helper so future additions stay in lockstep.

- **dirge-code#18 context::load_file** — silently swallowed
  `read_to_string` errors via `.ok()`. A permission-denied
  AGENTS.md looked identical to a missing file. Now emits a
  stderr warning naming the path + reason; still returns None
  so callers' behavior is unchanged.

725 plugin / 599 default pass. All build profiles clean.

## Remaining audit items (deferred to feature work)

- **dirge-code#8 LSP no crash restart**: needs broken-pipe IO error
  handling + exponential backoff. Touches manager state
  machine.
- **dirge-code#10 task tool fire-and-forget**: needs timeout + cleanup
  coordination via JoinHandle tracking.
- **dirge-code#25 MCP no reconnection**: similar architectural concern
  to dirge-code#8.
- **dirge-code#27 LSP didClose**: client lifecycle hook missing.
- **dirge-code#29 token estimation len/4**: needs per-provider usage
  extraction (Phase 6 work).
- **dirge-code#5 MCP shutdown**: rmcp Drop semantics need verification.
- **dirge-code#38/39/40 semantic test gaps**: get_symbol_body untested,
  list_symbols kind_filter untested, find_definition test
  vacuous. Sat down to add but each requires a fixture build.

Together with PR dirge-code#111 (10 code fixes), 17 of the 23 verified
items are now shipped. Remaining 6 are architectural or
test-infrastructure work better tackled as discrete PRs.

Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch pushed a commit to allen-munsch/dirge that referenced this pull request Jun 3, 2026
…1 leftovers (dirge-code#113)

Verified the 11 new round-2 findings and 6 round-1 leftovers
against the actual code. Shipping the 7 concrete fixes that map
to clear before/after behavior; the rest are deferred-architectural
or already-fixed-elsewhere.

## N1 — webfetch SSRF (CRITICAL)

`http://169.254.169.254/latest/meta-data/` (AWS metadata) and
all private/loopback ranges sailed straight through. An LLM that
gets prompt-injected into fetching a private IP could exfiltrate
IAM credentials, hit internal admin endpoints, or scrape internal
network maps.

New `validate_url_host_safety()` blocks:
- Hostname literal "localhost" / "ip6-localhost" /
  "ip6-loopback"
- IPv4 loopback (127/8), private (10/8, 172.16/12, 192.168/16),
  link-local (169.254/16 — cloud metadata), unspecified,
  broadcast, class E.
- IPv6 loopback, ULA (fc00::/7), link-local (fe80::/10),
  multicast, unspecified. Bracketed IPv6 (`[::1]`) is parsed
  correctly.

Opt-in escape hatch: `DIRGE_WEBFETCH_ALLOW_PRIVATE=1` for
dev workflows that need to hit localhost. Tool description
documents the env var so the agent can see it.

## N8 — webfetch body OOM (HIGH)

`resp.text().await` buffered the entire response into memory
before any truncation. A 500 MB page would OOM the agent
process. Now streams via `bytes_stream` with a 10 MiB cap;
bails at the cap.

## N2 — MCP result size cap (CRITICAL)

`McpTool::call` concatenated every `RawContent::Text` block
into `content` without limit. A 200 KB MCP response flooded
every subsequent turn until compaction. Cap aggregate at 256
KiB (≈65k tokens worst case); UTF-8 char-boundary slice;
append a clear truncation marker naming the offending
server::tool so the agent can adjust its calls.

## N3 — bash output cap (CRITICAL)

The UI's `render_tool_output` truncated the DISPLAY but the
full string was persisted to `ToolCallState::Completed` and
fed back to the LLM on the next turn. `cat /dev/urandom | head
-c 10M` would have shoved millions of tokens. Cap at 256 KiB
at the bash tool source, with a clear truncation message
pointing the agent at head/grep filtering.

## N5 — sandbox hardening (HIGH)

Added `--new-session` (drops the ability to gain new privs via
setuid) and `--unshare-user-try` (explicit user-ns isolation,
defensive against future bwrap default changes). `--unshare-all`
already covers most of this; the explicit flags pin the
guarantees.

## N7 — sanitize_output strips \\r (HIGH)

Previously `\\r` was preserved (originally for CRLF lines). A
bash tool printing progress with `\\rstep N/M` would move the
cursor to column 0 mid-line, overwriting chamber borders. Now
stripped (`\\n` and `\\t` still pass; display path
normalizes CRLF before reaching here).

## H10 — task subagent timeout

`tokio::spawn(btw_query(...))` with no timeout meant a stuck
provider would keep the task in `Running` forever. Cap at 10
minutes; on timeout, the task transitions to `Failed` with a
clear message and the system-reminder still fires.

## M22 — /prompt default help text

Behavior was fixed in PR dirge-code#111 (activates `default.md` if
installed). Help text still said "clear active prompt". Now
explains both cases.

## Verified false positives / non-issues this round

- **N4 sandbox --ro-bind / /**: by design — the sandbox is a
  WRITE guard, not exfiltration prevention. Defense for read
  access requires `--ro-bind` per path (much narrower scope
  + breaks read-only tooling like bash `ls`). Document as
  feature, not bug.
- **N6 sandbox current_dir fallback**: `unwrap_or_else(|_|
  ".".into())` is the standard fallback; `current_dir()`
  failing means cwd is deleted/inaccessible which is its own
  fatal-soon problem.
- **N9 token estimation len/4**: Phase 6 work; per-provider
  usage extraction needs distinct fix.
- **N10 sandbox /dev/tty**: `--dev /dev` mounts a tmpfs with
  only essential nodes, not the full host /dev. /dev/tty IS
  available to the sandboxed process so it can write to its
  own controlling terminal (needed for bash interactive
  features); not the host's /dev/tty0.
- **H17 DeepSeek auto-detect order**: confirmed false positive
  in earlier rounds — only fires when env var present.
- **H18 load_file path in error**: already fixed in PR dirge-code#112.
- **M19 semantic tools in BUILTIN_TOOL_NAMES**: semantic tools
  added separately, can't be shadowed by MCP collision.

## Tests

2 new regression tests:
- `validate_url_host_safety_blocks_ssrf_targets`: pins
  169.254.x, 127.x, RFC 1918, ::1, fc00::, fe80:: as refused;
  public IPs + domains pass; bracketed IPv6 parses correctly.
- `validate_url_host_safety_handles_malformed_hosts`: garbage
  hosts don't panic.

727 plugin / 601 default pass.

## Deferred to architectural follow-ups

- H8 LSP no crash restart — broken-pipe handling design
- N4 sandbox --ro-bind / — fundamental design tradeoff
- N9 token estimation — per-provider usage extraction

Co-authored-by: Yogthos <yogthos@gmail.com>
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.

1 participant