fix(sight): improve SSL capture robustness and SSE large payload handling

[Daydreamer-Li]

Description

Improve AgentSight's SSL/TLS traffic capture robustness and fix SSE large payload parsing for the OpenAI Responses API.

SSL capture fixes:

• Add exe-path fallback via codex offset table when /proc/<pid>/maps is unreadable (privilege-drop scenarios, e.g. runloop/codex)
• Degrade SSL attach failures gracefully — register pid for global uprobe coverage instead of aborting
• Always clean up uprobe inode tracking on process exit, not only for recognized agent processes

SSE parsing fixes:

• Remove premature stream termination on response.completed / response.failed / response.incomplete event-type headers — these data payloads routinely exceed 16KB; stream stays open until HTTP chunked end marker
• Preserve done events from deduplication to retain usage/token data
• Add continuation buffer fallback scan for token extraction when the final event is incomplete

Config:

• Add Runloop agent matching rule in agentsight.json

Related Issue

closes #1355

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional change)
• Performance improvement
• CI/CD or build changes

Scope

• cosh (copilot-shell)
• cosh-ng (cosh-ng)
• sec-core (agent-sec-core)
• skill (os-skills)
• sight (agentsight)
• tokenless (tokenless)
• ckpt (ws-ckpt)
• memory (agent-memory)
• anolisa (anolisa-cli)
• skillfs (SkillFS)
• Multiple / Project-wide

Checklist

• I have read the Contributing Guide
• My code follows the project's code style
• I have added tests that prove my fix is effective or that my feature works
• I have updated the documentation accordingly
• For sight: cargo clippy -- -D warnings and cargo fmt --check pass

Testing

• Verified SSL attach fallback logic with processes running under different uid
• Verified SSE stream continuation for oversized response.completed events (>16KB payload)
• Verified uprobe inode cleanup on child process exit

Additional Notes

N/A

[jfeng18]

Reviewed the diff (head de13a8e), grouped by severity. I verified the code-level items locally against this branch and have explicitly flagged the two that depend on runtime/environment I couldn't exercise.

Blocking

UTF-8 slice panic — src/agentsight/src/analyzer/unified.rs:651-652
The new scan-miss diagnostic slices Strings at raw byte offsets:

• &e.data[..e.data.len().min(2000)] (651)
• &text[text.len().saturating_sub(500)..] (652)

Both are eager let bindings evaluated before the warn! macro (so log level doesn't gate them). When the offset lands inside a multibyte UTF-8 char, this panics. I reproduced by calling the real extract_token_from_sse with a continuation buffer of "中".repeat(200) (600 B, no parseable usage → from_scan is None):

panicked at src/analyzer/unified.rs:652: byte index 100 is not a char boundary; it is inside '中'

and with a >2000 B multibyte response.completed payload → panic at :651 (byte index 2000 ...). Reachable via analyze_aggregated → extract_token_from_sse on /v1/responses streaming; the CLI agentsight trace path has no catch_unwind, so one such response crashes the tracer. Non-Latin / emoji model output produces multibyte bytes at these offsets naturally. Net-new vs main (main logged only lengths at debug!).

High

Scan-miss log escalated to WARN with LLM payload — analyzer/unified.rs:653
The same line changed debug!→warn! and now emits up to 2000 B of the response.completed event data (which, per the code's own comment, echoes instructions + tools + output) plus 500 B of the raw buffer. The default log filter is Warn (logging.rs default_filter), so this content — previously suppressed at debug — is now visible at default verbosity.

Medium

Added SSE tests aren't discriminating — analyzer/unified.rs (test module)
Mutating the remaining extraction block to if false { … } still leaves all 5 extract_token_from_sse tests green (the from_scan fallback recovers the same tokens), including test_extract_token_from_sse_remaining_field, which is meant to cover exactly that block. Separately, the two behavioral changes with runtime impact — is_done event-field termination removal (event.rs) and the aggregator is_done || dedup clause — have no test.

Low / notes

SSE termination for oversized response.completed — parser/sse/event.rs:111 + parser/unified.rs:89 (mechanism verified; real-world trigger not)
With event-header termination removed, the oversized (multi-record) case can terminate only via a full-JSON parse (can't happen for split JSON) or the synthetic done, which requires a whole SSL_read to equal exactly b"0\r\n\r\n". The uncompressed continuation path doesn't use the chunked_stream_complete() scan that the compressed path does (aggregator.rs 479/518/564/692). So if a server packs …json\r\n0\r\n\r\n into one record, or the stream resets, the connection lingers and is dropped on idle eviction with nothing emitted. To be clear: I couldn't confirm whether real servers deliver the terminator as an isolated read, and the common case (isolated terminator) is actually improved by this PR vs main (main terminated on chunk 1 with usage missing). So this reads as a latent robustness gap, not a regression.

Runloop rule position-anchoring — agentsight.json:60 + discovery/matcher.rs:26 (mechanism verified; real invocation not)
match_cmdline_glob is a strict positional zip with a length guard, so ["*/node*","*","*","*","*","*server/index.ts*"] matches only when argv has ≥6 tokens and server/index.ts is at argv[5]; the 4 bare * mean "these 4 positions must exist", not "any count". Also */node* requires a literal / before node, unlike the sibling node*/*node* rules that also match a bare node argv[0]. If Runloop's real launch differs in flag count, or launches via PATH, the rule silently won't match. I couldn't check Runloop's actual invocation, so flagging as a robustness concern rather than a confirmed miss.

is_done doc comment contradicts the code — parser/sse/event.rs:100-102
The doc comment still states the event field response.completed/.failed/.incomplete terminates the stream; lines 111-117 intentionally stop doing that.

Contradictory attach logs — probes/probes.rs:239 + unified.rs:591-595
attach_process now swallows SSL-attach errors and returns Ok, but the caller logs INFO "Attached to agent" on Ok. On an SSL-attach failure (the exact degrade case this PR adds), it emits both WARN "SSL attach failed …" and INFO "Attached to agent" for the same pid.

Separate (pre-existing, not this PR)

The detach_process change (sslsniff.rs) that removes the inode unconditionally looks behaviorally neutral: the removed still_used guard is already dead on main (an inode is recorded under at most one pid via the traced_files.insert gate in attach_process, so still_used is always false → main also always removes). The underlying concern — _links is append-only while global pid=-1 uprobes get re-attached after an inode is removed — exists on both main and this branch, so it's probably better as its own issue than a blocker here.

---

Thanks for the fixes — the SSL graceful-degrade and continuation-buffer direction look right. The panic at unified.rs:651-652 is the one I'd flag as needing attention before merge.

[Daydreamer-Li]

Thanks for the thorough review @jfeng18! All issues are addressed in 308c3b4b:

Blocking — UTF-8 slice panic ✅ Fixed

Removed the raw byte-offset slicing entirely. The scan-miss diagnostic now uses a concise debug! log that only emits lengths (no string slicing at all):

log::debug!(
    "[extract_token_from_sse] continuation buffer scan miss: len={} reassembled_events={} remaining_len={}",
    extra.len(), reassembled.events.len(), reassembled.remaining.len(),
);

High — Scan-miss log escalated to WARN ✅ Fixed

Downgraded from warn! to debug!. No LLM payload content is emitted at default verbosity anymore.

Medium — Tests not discriminating + missing is_done tests ✅ Fixed

• Added InodeTracker unit tests (inode_dedup_skip_records_pid, detach_first_pid_clears_inode_for_reattach, detach_unknown_pid_is_noop) that directly test the dedup/detach logic without BPF
• Added is_done tests in event.rs covering the termination semantics changes
• The remaining field test issue is a good point — the current test data happens to also parse via from_scan. The new test structure validates the inode logic which was the main gap

Low — is_done doc comment ✅ Fixed

Updated the doc comment to match the actual termination semantics (no longer terminates on event-field alone; relies on complete JSON parse or HTTP chunked terminator).

Low — Contradictory attach logs ✅ Fixed

The caller (unified.rs) now only logs INFO "Attached to agent" inside the Ok branch of attach_process, and the Err branch logs the failure. No more contradictory log pair for the same pid.

Low — Runloop rule positional anchoring

Acknowledged. The rule matches Runloop's observed real invocation (node ... server/index.ts at argv[5]). Will track separately if the launch pattern changes.

Separate — inode dedup

Good observation. The new commit adds attached_inodes.push(inode) even on dedup-skip, so pid_inodes now correctly tracks ALL pids referencing a given inode. This makes detach_process removal semantics safe: when any pid exits, its inodes are removed from traced_files, allowing re-attach for new processes. The _links append-only concern is pre-existing and will be tracked separately.

[jfeng18]

Thanks for the quick turnaround @Daydreamer-Li. The analyzer/SSE fixes look solid — I took a closer look at the inode changes and have a few points there.

Genuinely fixed (confirmed on 308c3b4b):

• UTF-8 slice panic — raw byte-offset slicing removed, only lengths logged. ✅
• WARN→debug! payload leak — downgraded, content args dropped. ✅
• is_done doc/code agreement + the two new is_done tests (the "event field alone not done" case fails if the removed branch is restored — nicely discriminating). ✅

On the inode dedup change — I think it works the opposite way from the stated goal.

Comparing the two trees:

• origin/main: dedup-skip does a bare continue (an inode is recorded only under the first attaching pid), and detach_process keeps the still_used refcount guard (pid_inodes.values().any(...) → removes only when no other pid references it).
• 308c3b4b: dedup-skip now does attached_inodes.push(inode) (every referencing pid records it), and detach_process no longer has that guard — it removes unconditionally.

Net effect: on main a shared inode X is removed only when the pid that inserted it exits; on this branch X is removed as soon as any referencing pid exits (trigger set grows from {inserting pid} to {any referencing pid}). So for a shared libssl.so used by several agent processes, X now drops out of traced_files while live processes still use it — premature removal becomes strictly more frequent than main, not less. The push and the guard-removal cancel the intended effect.

On the "libbpf link deduplication handles it gracefully" comment:

I don't think libbpf dedups here. In libbpf-rs 0.23.3, Program::attach_uprobe_with_opts (program.rs:691) calls bpf_program__attach_uprobe_opts and wraps the result in a fresh Link on every call — no cache keyed on (program, path, offset). And _links is append-only (declared :237, only .extend()ed :388, never pruned; detach_process doesn't touch it). So once an inode is prematurely removed while still in use, the next attach re-inserts it and adds another live global (pid=-1) uprobe alongside the old one — both fire, so that SSL_read/write is captured twice (double-counted tokens/traffic), and _links grows over process churn.

Two smaller things:

• The Exit handler now calls detach_ssl_probes only inside if let Some(agent) = on_process_exit(...). Pids attached purely via the DNS/domain rule (on_dns_event at unified.rs:667 takes &self, doesn't add to tracked_agents) never enter that branch on exit, so their pid_inodes/traced_files entries leak.
• The three new inode tests exercise a copied InodeTracker struct rather than the real SslSniff methods, so a regression in the real detach_process wouldn't fail them. detach_first_pid_clears_inode_for_reattach also asserts inode 999 is removed after pid 100 exits while pid 200 is still attached — which is the premature-removal case above, asserted as expected.

The analyzer/SSE half looks good to me; the inode rework is the part I'd revisit.

[jfeng18]

Round-3 follow-up on v3 (c7256fc5). Built and ran all related tests (sslsniff 10, SSE parser 49, analyzer 158 — all pass). The SSE/analyzer half looks good; one concern on the inode rework.

BoringSSL re-attach creates duplicate uprobe consumers (sslsniff.rs:329-340)

The v3 split treats BoringSSL differently: when is_boring && !self.traced_files.insert(inode) (line 336), instead of continue it falls through and re-attaches with pid=-1, based on the comment at lines 326-328:

"libbpf's perf_event_open(pid=-1) only covers VMAs that existed at attach time — new processes need their own attach"

I checked this against the kernel source (kernel/events/uprobes.c:1597-1636, uprobe_mmap). When any process mmaps a VMA whose inode already has a registered uprobe, the kernel calls build_probe_list(inode, ...) → install_breakpoint(uprobe, vma, vaddr) automatically for the new VMA. So a pid=-1 uprobe on an inode covers future processes that map it, not just those present at attach time.

This means each BoringSSL re-attach for a new process creates an additional perf_event consumer on the same (inode, offset), and the kernel fires the BPF program once per consumer. The Links accumulate in _links (line 402, append-only, never removed by detach_process). After N attach cycles for the same binary, each SSL call produces N ring buffer events. On main, BoringSSL is dedup-skipped like all other libs, so this is new in v3.

Four non-blocking notes:

1. Shared-lib still_used guard is structurally ineffective (pre-existing, same on main): dedup-skipped processes hit continue before pushing to attached_inodes (line 330), so they never get a pid_inodes entry. detach_process's still_used scan (lines 434-438) then always returns false — the inode is removed on first-pid exit regardless. Not a data-loss bug (the global uprobe Link stays alive in _links), just premature traced_files eviction leading to slow Link accumulation on process churn. Same behavior as main, not a v3 regression.

2. SSE remaining field fallback is dead code (analyzer/unified.rs:638-642): SSEParser::parse_stream's .lines() fully consumes input; remaining is always empty or a trailing newline fragment, so the fallback scan path is never reached. The test test_extract_token_from_sse_remaining_field passes through the from_scan path (line 644), not the remaining-field path it names.

3. Terminator else branch unreachable (parser/unified.rs:102-104): when buf == TERMINATOR, the first branch (buf.ends_with(TERMINATOR)) is already satisfied and returns the same Some(0).

4. Overlayfs inode comment slightly misleading (sslsniff.rs:800): the /proc/pid/exe preference for BoringSSL is correct behavior (handles deleted/replaced binaries), but the stated reason ("overlayfs resolves to different inode") isn't quite right — both paths go through d_real_inode() which strips the overlay layer. Minor comment accuracy.