feat(security): add egress logging inspector#8149
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef5c264df9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| egress_kind = dest.kind.as_str(), | ||
| destination = dest.destination.as_str(), | ||
| domain = dest.domain.as_str(), |
There was a problem hiding this comment.
This logs destination verbatim for every detected egress target, which can include credentials or secrets (for example basic-auth URLs, pre-signed query strings, or npm publish --token ... commands that are stored as the destination). In production telemetry this turns normal tool usage into secret leakage, so the logged value should be sanitized to host-level data (or otherwise redacted) before tracing::info!.
Useful? React with 👍 / 👎.
dorien-koelemeijer
changed the title
dorien-koelemeijer
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78f1d448b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| reason: format!( | ||
| "Egress destinations detected: {}", | ||
| destinations |
There was a problem hiding this comment.
The reason string embeds full destination values (including full URLs), and that value is later emitted by apply_inspection_results_to_permissions via reason = %result.reason in tool_inspection.rs, so credentials or signed query tokens are still logged even if tracing::info! fields are redacted. Any command like curl "https://api.example.com?token=..." will leak the token through this path.
Useful? React with 👍 / 👎.
|
|
||
| fn extract_domain_from_url(url: &str) -> Option<String> { | ||
| let after_scheme = url.find("://").map(|i| &url[i + 3..]).unwrap_or(url); | ||
| let authority = after_scheme.split('/').next()?; |
There was a problem hiding this comment.
Domain extraction splits only on /, so URLs without a path (for example https://example.com?sig=abc) treat example.com?sig=abc as the host. This corrupts the domain value used for egress telemetry/policy matching and can also copy sensitive query material into what should be a hostname field.
Useful? React with 👍 / 👎.
|
|
||
| static SSH_RE: OnceLock<Regex> = OnceLock::new(); | ||
| let ssh_re = SSH_RE.get_or_init(|| { | ||
| Regex::new(r"ssh\s+(?:-\w+\s+\S+\s+)*(?:\S+@)?([a-zA-Z0-9][\w.-]+)").unwrap() |
There was a problem hiding this comment.
The SSH regex assumes every -x flag is followed by a value, so common commands with valueless flags (ssh -v user@host) are missed, and mixed forms like ssh -N -L 8080:localhost:80 user@host can capture 8080 as the host. That produces incorrect or missing egress records for legitimate SSH usage.
Useful? React with 👍 / 👎.
|
|
||
| static DOCKER_RE: OnceLock<Regex> = OnceLock::new(); | ||
| let docker_re = DOCKER_RE.get_or_init(|| { | ||
| Regex::new(r#"docker\s+(?:push|login)\s+(?:--[^\s]+\s+)*([^\s'"]+)"#).unwrap() |
There was a problem hiding this comment.
The Docker pattern only skips long option tokens and not their values, and it ignores short flags entirely, so docker login --username alice ghcr.io records alice and docker login -u alice ghcr.io records -u as the destination instead of ghcr.io. This systematically misclassifies registry egress for common CLI invocations.
Useful? React with 👍 / 👎.
|
I think a reasonable idea - and any serious enterprise would love to use this IMO - so yeah, lets do this, doesn't seem to hurt things? |
| } | ||
| } | ||
|
|
||
| static GIT_SSH_RE: OnceLock<Regex> = OnceLock::new(); |
There was a problem hiding this comment.
would it lose much if things were a bit less bespoke - really anything potentially matching a host, could have shipped out for later analysis?
There was a problem hiding this comment.
Yeah, I like the idea, I'm just scared it will be too broad that way. I'll play around and see if iI can find a good way to do this that isn't too broad so the data doesn't become unusable.
There was a problem hiding this comment.
Made an attempt, it's a bit lame, but it's a bit more broad at least. I think maybe we proceed and see how it works and make changes as necessary?
DOsinga
added
the
needs_human
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a9a4d2f7f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| static NPM_PUBLISH_RE: OnceLock<Regex> = OnceLock::new(); | ||
| let npm_publish_re = NPM_PUBLISH_RE | ||
| .get_or_init(|| Regex::new(r"(?:^|[;&|]\s*|\n)\s*npm\s+publish(?:\s|$)").unwrap()); |
There was a problem hiding this comment.
The publish regex requires whitespace or end-of-input after publish, so valid shell one-liners like npm publish&&echo done or cargo publish;echo done are missed. Because these command forms are common in CI/scripts, the inspector will silently underreport package-registry egress for real publish operations. Allowing shell separators immediately after publish would preserve the false-positive protection while covering these cases.
Useful? React with 👍 / 👎.
dorien-koelemeijer
force-pushed
the
feature/egress-logging-inspector
branch
from
7a9a4d2 to
78f1d44
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78f1d448b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| static DOCKER_RE: OnceLock<Regex> = OnceLock::new(); | ||
| let docker_re = DOCKER_RE.get_or_init(|| { | ||
| Regex::new(r#"docker\s+(?:push|login)\s+(?:--[^\s]+\s+)*([^\s'"]+)"#).unwrap() |
There was a problem hiding this comment.
The Docker pattern requires a positional target after login, so docker login (without SERVER) is never captured. Docker’s own CLI behavior is that plain docker login authenticates to Docker Hub by default, so this misses a very common egress path and leaves the inspector silent for those sessions. Handle the no-argument login form by emitting docker.io (or equivalent Docker Hub host) when no explicit server is provided.
Useful? React with 👍 / 👎.
| }); | ||
| for cap in docker_re.captures_iter(command) { | ||
| let target = cap[1].to_string(); | ||
| let domain = target.split('/').next().unwrap_or(&target).to_string(); |
There was a problem hiding this comment.
The domain derivation uses target.split('/').next(), which treats unqualified images like alpine or myorg/app:latest as registry domains (alpine, myorg). Docker image references are [HOST[:PORT]/]... and default to Docker Hub (docker.io) when host is omitted, so this logic systematically misclassifies egress domains and can break any domain-based telemetry/policy derived from this field.
Useful? React with 👍 / 👎.
| ) || name.ends_with("__web_fetch") | ||
| || name.ends_with("__fetch") | ||
| || name.ends_with("__browser_navigate") |
There was a problem hiding this comment.
is_web_tool handles namespaced variants for web_fetch, fetch, and browser_navigate, but not for http_request. A tool named like mcp__<server>__http_request will bypass egress inspection entirely, creating a blind spot for outbound HTTP calls from namespaced MCP toolsets.
Useful? React with 👍 / 👎.
… bit of an expension at least
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa957d8adc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "shell" | "bash" | "execute_command" | "run_command" | "terminal" | ||
| ) || name.ends_with("__shell") | ||
| || name.ends_with("__bash") | ||
| || name.ends_with("__terminal") |
There was a problem hiding this comment.
is_shell_tool treats execute_command and run_command as shell tools only when the name is unprefixed, but extension tools are published as <extension>__<tool> (see extension_manager.rs tool renaming). That means calls like acme__execute_command/acme__run_command bypass this inspector entirely, so their outbound destinations are never logged. Add suffix checks for these two names (like the existing __shell/__bash/__terminal handling) to avoid this egress-inspection blind spot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3e00836aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| tracing::info!( | ||
| egress_kind = dest.kind.as_str(), | ||
| domain = dest.domain.as_str(), | ||
| destination = dest.destination.as_str(), | ||
| "egress destination detected" |
There was a problem hiding this comment.
This logs destinations during inspection, but inspection happens before permission/adversary decisions and before any tool execution (agent.rs calls inspect_tools before processing approvals/denials), so denied calls are still recorded as egress destination detected. In Approve/SmartApprove flows, a user can reject a command and still get a positive egress event, which makes the telemetry inaccurate for actual outbound traffic.
Useful? React with 👍 / 👎.
| static GENERIC_NET_CMD_RE: OnceLock<Regex> = OnceLock::new(); | ||
| let generic_net_cmd_re = GENERIC_NET_CMD_RE.get_or_init(|| { | ||
| Regex::new( | ||
| r"(?i)\b(fetch|nc|ncat|netcat|ftp|sftp|socat|httpie|xh)\b[^\n]*?\b((?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?\.)+[a-zA-Z]{2,})\b" |
There was a problem hiding this comment.
The extractor misses common commands like curl example.com and wget api.example.com: URL parsing requires ://, and the generic hostname fallback only keys off fetch|nc|ncat|netcat|ftp|sftp|socat|httpie|xh, excluding curl/wget. That causes systematic under-reporting of real shell egress in normal usage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60c0924c31
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| r"(?i)\b(fetch|nc|ncat|netcat|ftp|sftp|socat|httpie|xh)\b[^\n]*?\b((?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?\.)+[a-zA-Z]{2,})\b" | ||
| ).unwrap() |
There was a problem hiding this comment.
The generic network fallback regex only captures dotted hostnames with alphabetic TLDs, so common outbound commands like nc 10.0.0.8 443, socat TCP:127.0.0.1:9000 -, or sftp bastion are silently missed. In internal/VPC-heavy environments this creates systematic under-reporting of real egress destinations, which weakens the inspector’s stated observability and any downstream domain/IP policy logic derived from these records.
Useful? React with 👍 / 👎.
* origin/main: (85 commits) docs: standardize on ~/.agents/skills/ as canonical skill path (#8239) feat: bump versions (already published) (#8240) Revert "fix: rename bin for the tui (#8231)" (#8238) docs: update reusable recipes docs (#8232) docs: add GOOSE_SHELL env var (#8233) feat(security): add egress logging inspector (#8149) blog: Adversary Mode — A Second Pair of Eyes on Every Tool Call (#8220) fix: rename bin for the tui (#8231) feat: goose serve (#8209) refactor: make --text mode independent of the normal TUI (#8210) refactor: handle complex streaming/scrolling with better rendering (#8214) Add Inference Mesh settings tab (#8094) only run windows compile on main branch (#8216) used simplified privacy info modal and removed unnecessary components (#8200) removed unused welcome route (#8196) fix: use build_host_url for GCP Vertex AI location fallback (#8185) fix: remove ui/acp/.npmrc to allow auth token inheritance fix(openai): use safely_parse_json for streaming tool arguments (#8208) feat: add copilot-acp provider (#8154) chore(deps): bump path-to-regexp from 8.3.0 to 8.4.0 in /evals/open-model-gym/mcp-harness (#8178) ... # Conflicts: # ui/desktop/.gitignore # ui/desktop/src/components/Layout/CondensedRenderer.tsx # ui/desktop/src/components/Layout/navigation/ChatSessionsDropdown.tsx # ui/desktop/src/components/Layout/navigation/SessionsList.tsx # ui/desktop/src/components/ParameterInputModal.tsx # ui/desktop/src/components/parameter/ParameterInput.tsx # ui/desktop/src/components/recipes/RecipeActivityEditor.tsx # ui/desktop/src/components/recipes/shared/CreateSubRecipeInline.tsx # ui/desktop/src/main.ts
4 participants
Summary
Adds a new
EgressInspectorthat runs in observation-only mode alongside the existingSecurityInspector. It detects outbound network destinations in shell commands and web tool calls, logging them viatracing::info!without blocking anything. This gives visibility into what egress traffic goose is generating during a session.The inspector recognises the following egress patterns:
Tool coverage goes beyond shell commands — it also handles web fetch and browser navigation tools, matching both exact tool names and MCP-namespaced variants (e.g. mcp__developer__shell). The suffix-matching approach (e.g. ends_with("__shell")) means MCP servers that namespace their tools will be covered automatically without needing to enumerate every possible name. Multiple argument key names are tried (command, cmd, script, input for shell; url, uri, endpoint for web tools) to handle the variety of conventions used across different MCP servers.
Destinations are deduplicated within a single inspection pass so the same URL appearing multiple times in one session doesn't produce duplicate log entries. npm/cargo publish detection uses anchored regex rather than substring matching to avoid false positives from commands like echo "npm publish".
Each detected destination is logged with egress_kind, domain, and destination fields.
Note: the destination field may contain sensitive data (credentials embedded in URLs, internal hostnames). Sanitisation before any external log export (e.g. OTLP) should be handled at the transport layer and is out of scope for this PR (another PR will be opened shortly to address this).
Testing
Unit tests cover all supported egress pattern types — URLs, git SSH remotes, S3, GCS, SCP, rsync, SSH, Docker, and npm/cargo publish — including false positive cases for the publish detection.
End-to-end testing via
just run-uiwas attempted but the server's file logger does not currently write to disk (pre-existing issue, not introduced by this PR). Tracing output is available on stderr when running goosed agent directly, which confirmed the inspector is wired into the pipeline correctly. Full end-to-end validation of live egress detection is deferred until the logging infrastructure issue is resolved.