Skip to content

feat(diff-logs): report connections that are allowed and then fail as TLS trust problems - #24

Merged
ilanh merged 1 commit into
mainfrom
fix/diff-logs-tls
Nov 3, 2026
Merged

ilanh merged 1 commit into
mainfrom
fix/diff-logs-tls

Conversation

@ilanh

@ilanh ilanh commented Nov 3, 2026

Copy link
Copy Markdown
Owner

What and why

OpenShell can allow a connection and the client then drops it because it does not trust the sandbox's TLS-termination CA (Rust reqwest with bundled webpki roots, pinned certificates, some Java truststores). The log shows NET:OPEN ... ALLOWED immediately followed by NET:FAIL [LOW] <synthetic ip>:443. diff-logs used to report 0 suggestions for that.

  • Parses NET:FAIL lines (shorthand) and OCSF JSON events with activity_id 4. New Action::Failed, and a failed count in the JSON report.
  • A fail within 10 log lines after an ALLOWED net event of the same log is attributed to that event's host, port and binary (the FAIL line only carries the synthetic IP).
  • New suggestion kind tls_failure ("Allowed, then failed (TLS)") with the hint: the client most likely does not trust the OpenShell CA; use tls=skip (loses L7 rules) or make the client trust the CA bundle. It points at the existing directive when there is one.
  • If the directive already has tls=skip, the finding is a manual note saying it is not a certificate problem.
  • It never writes a directive (--write leaves files alone for this kind), because the fix is an attribute on an existing line.

Tests: tests/diff_logs_tls.rs (shorthand and OCSF JSON, tls=skip case, --write no-op). The anthropic-proxy-rs sandbox log kept in the candidates folder no longer contains the NET lines, so I could not replay the real log; the line format follows OpenShell's shorthand formatter (crates/openshell-ocsf/.../shorthand.rs) and the report.

TOOLCHAIN=1.94 scripts/ci-local.sh and stable clippy pass. Independent of the other PRs (new test file; the diff-logs source edits are away from the lines other PRs touch).

Closes #12

@ilanh
ilanh force-pushed the fix/diff-logs-tls branch from a601b7e to 7eac5ae Compare November 3, 2026 08:06
@ilanh
ilanh merged commit d5e2e53 into main Nov 3, 2026
2 checks passed
@ilanh
ilanh deleted the fix/diff-logs-tls branch November 3, 2026 08:10
ilanh added a commit that referenced this pull request Nov 3, 2026
… scripts out of placement (#26)

## What and why

Two `diff-logs` problems found when the probe driver lived inside the
repo:

- **Wrong root from a subdirectory.** `cd openshell &&
openshell-annotate diff-logs sandbox-logs.txt` scanned only that
directory (0 stale, wrong `place:`). When `--root` is not given,
`diff-logs` now walks up to the nearest `openshell-annotate.toml` (like
git), prints `note: using repository root ...`, and loads that config.
The log paths you pass are still resolved from where you run it. An
explicit `--root` other than `.` is never second-guessed. (The other
commands take explicit path arguments relative to the working directory,
so I left their root handling alone.)
- **Probe scripts used as placement targets.** `diff-logs --write`
inserted directives into `openshell/exercise.sh` because it mentions the
host. New `[diff-logs] exclude` patterns (same literal syntax as
`[scan].exclude`) keep files from being picked as the place for a
suggestion, while directives inside them still count. The
`--observed-file` is always skipped.

Docs: README config block, `--root` help (CLI.md regenerated).

Tests: `tests/diff_logs_root.rs` (subdirectory run, `--write` from a
subdirectory, exclude vs no exclude, observed file never a target).
`TOOLCHAIN=1.94 scripts/ci-local.sh` and stable clippy pass.

Ordering: edits the `diff-logs` arm of `main.rs` and `find_mention` in
`diff_logs.rs`; the TLS PR (#24) edits other parts of `diff_logs.rs`, so
a rebase of the second one to merge may be needed.

Closes #11

Signed-off-by: Ilan Harel <[email protected]>
Co-authored-by: Ilan Harel <[email protected]>
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.

diff-logs does not report TLS failures after an ALLOWED connection

1 participant