Conversation
…on and the synchronous sink (Spec 107 PR-D)
…-validated builder, synchronous sink (Spec 107 PR-D)
…ation, nested observer and error classes (Spec 107 PR-D)
… nested code_execution observer, error classes (Spec 107 PR-D)
T096-T119: Edition-neutral audit line emission at authorization and tool-call decision points.
- internal/audit/error_class.go: ErrorClass enum (upstream_error, upstream_unavailable, validation, sanitisation, cancelled, internal) with errors.Is/As routing and ErrorClassOf() type switch
- internal/jsruntime/runtime.go: AuthzObserver interface (report cached decision), AuthzGateReport with ParentID, ExecutionOptions.{AuthzObserver, ParentID} for nested tracking, nestedAuthzObserver implementation
- internal/server/audit_funnel.go: auditDispatch (attempt wrapper), installAuditAttempt (RFC 8785 JCS arg hash, mount/source/origin/profile derivation), auditCallerFromContext (full table: stdio, socket, agent_token, session_admin, anonymous), auditAuthz (deny reason from telemetry.BlockReason, disclosed:false for scopes), auditToolCall (pairs authz, error_class routing), auditToolCallShed (rejected outcome)
- internal/server/mcp.go, mcp_routing.go, mcp_code_execution.go: ctx as first param to emit* activity functions; handleCallToolVariant surface/intent gates; code_execution bridge with parent_id + nested observer
- internal/server/server.go: ServerOption pattern, WithAuditSink, stdioAuthContext tags
- internal/server/serveredition_wire.go: AuditSink dependency wiring
- internal/server/audit_funnel_test.go: 11 tests covering allow/deny, hidden-server scope, intent rejection, nested calls, limiter shed, stdio/socket callers, schema validation
- activity_result_status_test.go, code_exec_activity_test.go, preflight_telemetry_test.go: statusArgIndex 5→6, ctx passed to emit functions
All audit lines validated against contracts/audit-line.schema.json; no secrets/tokens in clear; one authz per attempt, one tool_call per call.
…ults, doctor finding, write-failure metric, settings wiring (Spec 107 PR-D) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…sertion, release notice (Spec 107 PR-D)
T116: ran scripts/dev-server-edition.sh --phase d end to end, plus manual extensions for cases the script doesn't cover (sentinel in a caller-supplied tool name, native stdio transport rules, unwritable path). Fixed two rig bugs found along the way (not in the audited feature code): the scratch config left fixture servers quarantined, and the isError=null vs "false" assertion didn't accept the omitempty case. Documented a pre-existing, out-of-scope discrepancy: `--listen ""` doesn't actually reach native stdio through `mcpproxy serve` (Validate() resets it to the HTTP default); `--listen ":0"` does. T117: re-ran the full gate set since the adversarial-review commit changed code after the prior recording. All green; two transient flakes under heavy concurrent load (a Docker-status test, a launcher-lifecycle respawn test) confirmed non-reproducing / out of this PR's scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Spec 107's four PRs (A, B, C, D) are all open; PR-D is #1296. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
52a10e6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d0c973e5.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://107-d-audit-line.mcpproxy-docs.pages.dev |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 35242884720 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
opencode CLI (gpt-5.6-sol / gpt-5.6-terra) reviewed the PR-D diff in 5 chunks; all 5 returned findings (16 total). 8 verified genuine and fixed: NaN/Infinity silently hashed as JSON null instead of refused (canonical.go), client.version unmasked in the audit line, a short sink write not counted as a failure, a malformed args_json dispatch producing no audit line at all, work_session_id never stamped on the attempt, auth_event lines never carrying client.ip, two missing FR-014 startup notices, and a missing sidebars.js entry for the new audit-log doc. 3 rejected as false positives with evidence (redact.go patterns are a superset of the log sanitizer's, not narrower; no request_id==transport_request_id binding rule exists in the contracts doc; the unknown-server fail-open in mcp_code_execution.go is a documented deliberate design choice). 5 more confirmed genuine but deferred to a later round given their blast radius (JCS float formatting, per-kind caller-identity validation, auth_event flags vocabulary, Attempt.Operation sourced from the caller's variant instead of the resolved target tier, and a batch-cancellation gap in the tool_call/authz pairing) — all documented in verification.md. Full round detail, rejection evidence and verification commands in specs/107-server-edition-sso-hardening/verification.md under PR-D / Cross-review / Round 1.
Fixes 8 findings from round 2 (opencode gpt-5.6-sol/terra, 5 chunks), three of them round-1's own deferred list independently re-confirmed: - ES6 Number::toString fixed/exponential threshold in formatNumberJCS (args_sha256 could diverge from a real JCS reference implementation) - length-cap the per-field audit redaction pass (unbounded client.name could drop a required line at the sink's record-size limit) - Server.ReplayToolCall wrote zero audit lines (bypassed the funnel entirely via runtime.ReplayToolCall's direct managed-client call) - malformed args_json recorded authz allow before the remaining gates ran; now authz deny (reason: other) - audit line operation now reflects the target tool's real tier, not the caller's chosen call_tool_* door - batch dispatch no longer skips the audit-attempt bridge on an already-cancelled execution context - UpdateUserLogin returns the record a subject_mismatch/user_disabled refusal was decided against, closing a race that could silently drop auth_event's required user_id - HandleLogin now flags redirect_rejected on a pre-redirect failure, not only via the callback's stored pending state - EffectiveAuditLog now honours an explicit audit_log block on the personal edition; only the absent-block default is edition-keyed See specs/107-server-edition-sso-hardening/verification.md for the full per-finding table and verification commands.
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
2 participants
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 an attributable, edition-neutral JSONL audit line at every authorization
decision and tool-call funnel, plus a login/logout
auth_eventline for theserver edition's OAuth front door (Spec 107 PR-D, US3, FR-012–FR-019,
FR-039 part 4, FR-043(b,j), SC-003, SC-009).
internal/audit: JCS (RFC 8785) canonicalisation over sanitised args →args_sha256; three typed constructors (NewAuthz,NewToolCall,NewAuthEvent) so a forbidden field per event type doesn't compile; asynchronous, mutex-guarded sink (file via
lumberjack, or raw stdout)with an always-on write-failure counter; per-field sanitisation of every
caller/operator-controlled string (fixed-prefix credential patterns only —
never the generic high-entropy rule, which would mask
args_sha256/email_hashthemselves) plus a whole-line defence-in-depth pass that isasserted to be the identity on every constructor's output.
internal/server:authz+tool_calllines at thecall_tool_*variants, direct dispatch, the REST direct path, and the nested
code_executionwrapper (parent_id+surface:code_execution); calleridentity derived for every kind (API key, socket, anonymous, agent token
owned/ownerless,
session_admin, internal, and native stdio, whichneeded a new
transport.ConnectionSourceStdiotag since stdio installs anadmin context with no connection source today).
internal/serveredition/auth: oneauth_eventline per terminal loginattempt (every FR-013 reason) and one per logout, with stage-dependent
identity (
user_idonce the store is reached,email_hashfor averified-but-unresolved email, neither pre-identity).
internal/config:AuditLogConfig+EffectiveAuditLog(cfg, transport)encoding the stdio rule (FR-014: stdout can never double as the sink when
stdout carries JSON-RPC — absent block under stdio → disabled + WARN,
explicit stdout-only under stdio →
StartupErrorexit 4), validationreachable from boot/PATCH/apply alike, hot-reload restart-pinning, a
mcpproxy_audit_write_failures_totalcounter and a doctor finding.docs/features/audit-log.md,docs/schemas/audit-line-v1.schema.json,docs/operations/deploying-for-a-team.md, release notice bullets.Stacked on #1293 → #1292 → #1287 — merge in order.
Example lines
{"event":"authz","decision":"allow","server":"a","tool":"echo","reason":"none","operation":"read","caller":{"kind":"agent_token","user_id":"01M...","user_email":"alice@example.com","token_name":"t1","token_prefix":"mcp_agt_9f6b"},"request_id":"...-a-echo-1","schema_version":1} {"event":"tool_call","outcome":"success","server":"a","tool":"echo","args_sha256":"7804fe...","args_bytes":52,"caller":{"kind":"agent_token","user_id":"01M...","user_email":"alice@example.com"},"request_id":"...-a-echo-1","schema_version":1} {"event":"authz","decision":"deny","server":"b","tool":"echo","reason":"token_scope","disclosed":false,"caller":{"kind":"agent_token","...":"..."},"request_id":"...-b-echo-3","schema_version":1} {"event":"auth_event","surface":"login","reason":"ok","caller":{"kind":"session_user","user_id":"01M...","role":"user","provider":"oidc"},"schema_version":1}Config keys
{ "audit_log": { "enabled": true, "stdout": false, "path": "/var/log/mcpproxy/audit.jsonl", "max_size_mb": 50, "max_backups": 10, "max_age_days": 90, "compress": true } }Defaults: personal edition
{enabled:false}; server edition with the blockabsent
{enabled:true, stdout:true}— except under the native stdiotransport, where stdout carries JSON-RPC and the default resolves to
{enabled:false}with one startup WARN namingaudit_log.pathinstead. Anexplicit
stdout:trueunder stdio is refused at boot (StartupError,exit code 4), never silently disabled. An unwritable
pathis also exitcode 4, caught by a pre-flight open/close probe before the rotating writer
installs (
lumberjackopens lazily and would otherwise swallow it).Verification gate table (full detail in
verification.md)-tags server)go vet×2go test -racenon-server (excl.internal/server)internal/server, personal tags, CI-skip regex-tags server -racego test ./cmd/...,./tests/oauthserver/...make swagger-verify+TestContractsInSyncpython3 scripts/gen-roadmap.py --check./scripts/test-api-e2e.sh+ new audit assertions (T113)dev-server-edition.sh --phase d+ manual extensions)auth_event, schema validation viaTestExternalJSONLValidates, stdio WARN/exit-4 rules, unwritable-path exit 4Two rig bugs found and fixed while running T116 (in
scripts/dev-server-edition.sh,not in the audited feature code): the scratch config left fixture servers
quarantined, and an
isErrorassertion didn't accept theomitemptycase.One pre-existing, out-of-scope discrepancy documented but not fixed:
--listen ""does not actually reach native stdio through
mcpproxy servetoday(
Config.Validate()resets an emptyListenback to the HTTP default) —--listen ":0"is the only path that works; flagged for separate follow-up.Benchmark deltas (SC-009, same-tree audit-on vs audit-off, bound = max(10%, 5ms))
call_tool_read(admin)retrieve_tools(admin)tools/list(admin)retrieve_toolsscoped vs admin