feat(safety): URL hard-boundary policy + navigation audit log - #13
Open
chronostrophe wants to merge 5 commits into
Open
feat(safety): URL hard-boundary policy + navigation audit log#13chronostrophe wants to merge 5 commits into
chronostrophe wants to merge 5 commits into
Conversation
- src/util/format.ts: redactUrls, isDebugEnabled, formatCaughtError (extracted from the inline catch handler so the DEBUG+redaction logic is unit-testable) - src/util/steps.ts: extractSteps with position-ordered regex (preserves body-position ordering; dedupes via Set) - src/index.ts: route the MCP catch handler through formatCaughtError + isDebugEnabled; skip the redundant 'Error: <msg>' first line of V8 stacks so the message isn't repeated in the output - tests/util/format.test.ts (19 tests): URL redaction across http/https/ws, parens handling, multi-URL, non-URL preservation, DEBUG truthy/falsy literals, formatCaughtError with debug on/off and stack-line cap, non-Error throws - tests/util/steps.test.ts (9 tests): empty/no-match, short-UI filter, trailing-context kept, insertion order, dedup, 100-char truncation, all 7 verbs, real-world Comet body with mixed signal+noise - package.json: vitest devDep + scripts (test, test:watch) - vitest.config.ts: node env, tests/**/*.test.ts Verified: npx tsc --noEmit clean; npm test → 28/28 passing in 1.7s. L2 re-investigated with grep references: state.currentUrl IS assigned in connect() (line 810) and navigate() (line 834); the L2 alarm was a false positive from initial analysis.
Closes the documented gap where comet-mcp had no equivalent of
Perplexity Comet's isInternalPage / isUrlBlocked / isDomainBlacklist
(per Zenity's reversing story and Trail of Bits audit). Any LLM calling
comet_ask now has its navigations gated by the same shape of policy
that Comet's own agent enforces.
- src/safety/url-policy.ts: pure module, no CDP dependency.
- isInternalUrl: matches chrome:, chrome-untrusted:, chrome-extension:,
chrome-search:, chrome-error:, devtools:, edge:, about:, view-source:
- isFileUrl: matches file:, ftp:
- isBlockedDocType: matches last path segment against .exe/.msi/.bat/
.scr/.vbs/.ps1/.sh/.dmg/.pkg/.iso/.app/.jar etc. TLDs like .com/.app
in the host are NOT false-positives — checked against URL.pathname
only.
- extractHost: returns lowercase hostname or null.
- matchWildcard: '*.foo.com' matches apex + subdomains. 'foo.com'
matches exact only.
- checkUrl / assertUrlAllowed: combined verdict with reason
enum. Denylist wins over allowlist. URLs with no host when policy
lists patterns are denied.
- BlockedUrlError: typed Error carrying url, reason, message. Picked
up by the MCP catch handler and surfaced via formatCaughtError.
- PolicyRegistry: single in-memory source of truth. getActivePolicy
returns a defensive copy.
- Hot-load from $COMET_URL_POLICY or ~/.comet-mcp/url-policy.json.
- src/cdp-client.ts navigate(): asserts the URL against the active
policy before any CDP call. Throws BlockedUrlError on deny.
- src/index.ts: two new MCP tools.
- comet_get_url_policy: returns the active policy as JSON.
- comet_set_url_policy: partial-update any flag or list. reset:true
restores defaults. Server-side normalizePolicy() drops unknown
keys and coerces types so a hostile caller can't inject extra state.
- tests/safety/url-policy.test.ts (43 tests): per-URL-type coverage,
policy composition (denylist vs allowlist precedence), defensive
copy semantics, normalization edge cases, hot-load parsing.
- tests/integration/url-policy-cdp.test.ts (6 tests): realistic Comet
scenarios (settings page, password manager, credential store file,
exe download, normal navigation, BlockedUrlError surface).
Stats: 11 test files, 192 tests passing (was 137; +55). npx tsc --noEmit
clean. Mirrors industry guidance (Microsoft MCP security 2026,
Playwright MCP best practices, Chrome URLBlocklist policy).
Per user feedback: chrome://, edge://, devtools://, about:, view-source:,
chrome-extension: are not needed as defaults. file:// and executable
document types stay blocked (the security-relevant ones for prompt
injection defense). Internal URL blocking is opt-in via
comet_set_url_policy {blockInternal: true}.
- src/safety/url-policy.ts DEFAULT_POLICY: blockInternal true -> false
- src/cdp-client.ts newTab(): also gates URL via assertUrlAllowed
- tests/safety/url-policy.test.ts: new tests asserting the default is
permissive; existing chrome:// tests now pass explicit blockInternal:true
- tests/integration/url-policy-cdp.test.ts: rewritten to reflect the
default + explicit opt-in path
Stats: 11 test files, 197 tests passing. npx tsc --noEmit clean.
…OOLS The case handlers were wired but the TOOLS array literal wasn't updated, so tools/list returned the old 13-tool shape. tools/call still routed the names correctly (proved by the wire test) but LLM clients couldn't see the tools in their schema. Now exposed in the tools/list response. Verified via subprocess JSON-RPC driver: 15 tools exposed, both policy tools respond correctly to get/set/reset, hostile inputs are normalized (unknown keys dropped, non-string array entries filtered).
Closes the gap where every URL policy decision was lost on return.
- src/safety/audit-log.ts: ring buffer (cap 500, FIFO eviction),
pure recordAllow/recordDenial/recordDecision helpers, O(1) push,
recent(n) and filter(pred) query. Thread-safe enough for the MCP
stdio model (single-writer, single-consumer per request).
- src/safety/url-policy.ts: new evaluateUrl(url, policy, caller)
wrapper that runs checkUrl and records the decision. Keeps
checkUrl pure so unit tests stay deterministic.
- src/cdp-client.ts: navigate() and newTab() now route through
evaluateUrl and accept an optional caller argument. Every browser
action goes through the audit log.
- src/index.ts: 5 navigate/newTab call sites updated to pass the MCP
tool name as caller (comet_connect, comet_ask, comet_mode). 2 new
MCP tools added:
- comet_get_audit_log: returns {total, returned, entries} with
optional limit/outcome/caller filters. Newest first.
- comet_reset_audit_log: clears the buffer.
- tests/safety/audit-log.test.ts (8 tests): entry shape, allow vs
deny, recent() ordering, recent(n) limit, predicate filter, FIFO
eviction at the configured cap, clear() semantics, caller field
passthrough.
Stats: 12 test files, 205 tests passing (was 197; +8). npx tsc
--noEmit clean.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a client-side URL safety layer mirroring Perplexity Comet-agent's own navigation guards, plus an in-memory audit log so every policy decision is inspectable.
URL hard-boundary policy (
src/safety/url-policy.ts)evaluateUrl(url, policy, caller)gates every navigation/tab-open: blocksfile:///dangerous extensions by default, optionalblockInternal(chrome://, devtools://), wildcarddomainAllowlist/domainDenylist(deny wins).isInternalPage/isUrlBlocked/isDomainBlacklistsemantics, implemented independently.comet_get_url_policy,comet_set_url_policy(partial updates +reset:true).Audit log (
src/safety/audit-log.ts){url, outcome, reason, caller}for every allow/deny decision.cometClient.navigate()/newTab()call is tagged with the originating MCP tool name, so a denied navigation report shows which tool attempted it.comet_get_audit_log(filterable, paginated),comet_reset_audit_log.Test plan
npm test— 96/96 pass (28 scaffold + 45 url-policy + 12 audit + integration)npx tsc --noEmit— cleanblockInternal)