Repository navigation
Da-2038 auditability phase1 synced - #291
Open
AayushTyagi1 wants to merge 43 commits into
Open
AayushTyagi1 wants to merge 43 commits into
AayushTyagi1 wants to merge 43 commits into
Conversation
Mechanical conflict resolution only; the audit feature is ported onto the new multi-package layout in the follow-up commit. Resolutions: * tools/query.py -> tools/operational/query.py. Rename tracked by git; the audit hunks merged cleanly and only the import block conflicted. Imports re-pointed at utils/operational/* and OPERATIONAL_LOGGER_NAMESPACE. * tools/server.py deleted upstream. get_server_configuration_status now lives in the shared tools/status.py, so the audit block moved there — a better home, since it reads the lifespan context and the process-wide logger and names no SDK. * utils/context.py, utils/__init__.py: additive. Kept get_audit_config alongside main's server_id/server_name; dropped get_cluster_connection, which main moved to utils/operational/context.py. * utils/elicitation.py, utils/scope_enforcement.py: import blocks only. MCP_SERVER_NAME -> LOGGER_NAMESPACE. Audit hooks unchanged. * mcp_server.py: took main's version wholesale. It is now a thin host; the audit CLI options and lifespan wiring relocate to utils/cli_params.py and core/app.py in the follow-up.
Main split the repo into service packages and added a second server. Three consequences for auditing, all handled here. **Auditing is now opt-in per server.** ServerSpec gains `audit_package`, naming the Tier-2 block a server's tool calls are booked against. Operational sets "operational"; Operational Insights leaves it None and is not audited. This is not a shortcut: the two servers collide on five tool names (create_index, list_indexes, get_collections_in_scope, get_schema_for_collection, get_server_configuration_status), so auditing OI against operational's block would give two different operations the same event id — the one failure the two-tier catalogue exists to prevent. OI's block is left unallocated rather than spent on a tool surface that is still moving; an id, once records exist against it, can never be renumbered. **The catalogue and classification take a package dimension.** CATEGORY_SLOTS, RETIRED_SLOTS and TOOL_CLASSIFICATION are keyed by service package, and SERVICE_PACKAGE_BLOCKS maps package -> block base. Adding OI later is a table, a block and one spec field, and renumbers nothing. **A search category for the FTS tools, at slot 8.** Ids 61496 / 61528. Slots 6 and 7 (retired vector/analytics) stay retired: reuse was permitted by the documented rule — neither ever emitted a record — but declined, because two dead numbers cost nothing in a 16-slot block while a number that has meant two things is a trap for anyone reading an old rule. next_free_slot no longer offers retired slots, so that stays a deliberate decision rather than a default. Six new operational tools classified, taking the table to 40/40 against the registered set: list_fts_indexes, get_fts_index_definition, run_fts_query (search read), get_cluster_metrics and discover_tool_input_values (cluster read), get_index_stats (index read). All ship read-only; the search *write* id exists for when FTS index management lands. Wiring follows main's seams rather than working around them: * CLI flags -> core/cli/options.py as an `audit_options` stack, resolved by CliParams.audit, so both subcommands accept them and neither retypes them. * Lifespan, middleware registration and the startup/shutdown records -> core/app.py, which takes `audit_config` for the same reason it takes `logging_config`: a host that resolves configuration without a command line can populate it without adopting this repo's CLI. * The status-tool audit block -> the shared tools/status.py. Also satisfies two of main's guards rather than bypassing them: the five audit loggers are added to the logger-name snapshot, and `audit_config` is classified in the settings test as deliberately unreported, since get_server_configuration_status and the `server configuration` record both already carry it in full. 1059 unit tests pass; ruff check and format clean.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| await client.call_tool("get_document_by_id", {"bucket_name": "b2"}) | ||
| finally: | ||
| server.should_exit = True | ||
| await serving |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
SSE identity attribution, startup lifecycle, SQL++ classification, and sink failure handling can produce misleading records or stop audit delivery.
Review effort: Balanced
Findings: 3
Open (7)
What changed in this PR
Introduces phase-one audit logging for the operational MCP server, covering lifecycle, authorization, guardrail, and tool-call events.
Changes:
- Adds audit configuration, event cataloguing, identity resolution, middleware, and rotating JSONL storage.
- Integrates audit hooks into authentication, scope enforcement, confirmations, SQL++, and status reporting.
- Adds extensive unit and transport-level audit tests.
| File | Description |
|---|---|
tests/unit/test_settings_classification.py |
Classifies audit settings reporting. |
tests/unit/test_logger_names.py |
Registers audit logger names. |
tests/unit/test_audit_state.py |
Tests request-local audit state. |
tests/unit/test_audit_sink.py |
Tests writing, rotation, and concurrency. |
tests/unit/test_audit_record.py |
Tests audit serialization. |
tests/unit/test_audit_middleware.py |
Exercises middleware end to end. |
tests/unit/test_audit_integration_points.py |
Tests enforcement and identity hooks. |
tests/unit/test_audit_config.py |
Tests configuration resolution. |
tests/unit/test_audit_classification.py |
Validates tool classification. |
tests/unit/test_audit_catalog.py |
Validates stable event IDs. |
src/mcp_server.py |
Passes audit configuration into applications. |
src/cb_mcp/utils/scope_enforcement.py |
Records scope denials. |
src/cb_mcp/utils/elicitation.py |
Audits confirmation outcomes. |
src/cb_mcp/utils/context.py |
Exposes audit configuration context. |
src/cb_mcp/utils/constants.py |
Defines audit defaults. |
src/cb_mcp/utils/cli_params.py |
Resolves audit CLI parameters. |
src/cb_mcp/utils/__init__.py |
Exports audit utilities. |
src/cb_mcp/tools/status.py |
Reports audit status and counters. |
src/cb_mcp/tools/operational/query.py |
Classifies and correlates SQL++ requests. |
src/cb_mcp/servers/operational/spec.py |
Enables operational-server auditing. |
src/cb_mcp/core/spec.py |
Adds audit-package metadata. |
src/cb_mcp/core/cli/options.py |
Adds audit CLI options. |
src/cb_mcp/core/app.py |
Manages audit lifecycle and middleware. |
src/cb_mcp/auth.py |
Audits rejected tokens. |
src/cb_mcp/audit/state.py |
Implements request audit state. |
src/cb_mcp/audit/sink.py |
Implements asynchronous rotating storage. |
src/cb_mcp/audit/record.py |
Defines the audit wire format. |
src/cb_mcp/audit/middleware.py |
Emits request audit events. |
src/cb_mcp/audit/identity.py |
Resolves caller identities. |
src/cb_mcp/audit/exceptions.py |
Defines auditable refusal exceptions. |
src/cb_mcp/audit/emitter.py |
Implements the process audit logger. |
src/cb_mcp/audit/descriptor.json |
Publishes the event catalogue. |
src/cb_mcp/audit/config.py |
Parses and snapshots audit configuration. |
src/cb_mcp/audit/classification.py |
Maps tools to audit categories. |
src/cb_mcp/audit/catalog.py |
Defines event blocks and IDs. |
src/cb_mcp/audit/__init__.py |
Exposes the audit package API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…om/Couchbase-Ecosystem/mcp-server-couchbase into DA-2038-auditability-phase1-synced
Conflicts were additive on both sides and resolved as unions: the audit CLI stack alongside the embedding one, the operational spec's settings key tuples, and server.json's env-var lists for the two operational packages. Vector search arrives with tools the classification table did not know, so run_vector_search books as a query read and run_search_vector_search as a search read, matching the service each one calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
create_query_index takes the index write id (61524) its deprecated predecessor create_index already used, so superseding the tool does not move the id a SIEM rule watches. The N1QL Admin REST tools are booked as performance rather than query: 'query read' means a caller ran a statement, and a reviewer reconstructing what was executed must not find listings of other callers' in-flight requests mixed into it. delete_active_query is the first performance write (61525), taking an id the catalogue already reserved. Without these entries the fail-closed path recorded all four against a write id flagged unclassified, which is why two classification tests were failing on the branch tip.
Operational Insights is now audited under its own Tier-2 block, 0x10000.
It shares the category axis with the operational server but not the block:
the two collide on five tool names -- get_server_configuration_status,
create_index, list_indexes, get_collections_in_scope,
get_schema_for_collection -- so booking its calls against the operational
block would file them under ids that mean a different server. All 13
registered tools are classified; the roadmap's groupings map onto the
shared axis rather than replacing it (Data Model & Schema Discovery and
Metadata Introspection -> schema, Async Request API -> query, Query
Monitoring -> performance).
A package omits the slots it has no tools for rather than renumbering, so
Operational Insights uses 1/3/4/7 and the rest keep their meaning.
New category (MCP server status and configuration) takes slot 7
in both blocks: 61495/61527 and 65591/65623. That slot held the retired
category. Reviving it is permitted only because
never had a tool classified to it and so never emitted a record -- there
can be no SIEM rule for the old meaning. next_free_slot still declines to
offer retired slots, so this was an explicit decision rather than one
inherited from a helper; a test now asserts the precondition.
Moved into the category by product decision:
get_server_configuration_status, discover_tool_input_values -> config
get_cluster_query_vitals (from performance), get_index_stats (from index)
-> cluster, which now means Cluster Setup & Health
ALL_IDS and FILTERABLE_IDS span every package, so an operator sharing one
CB_MCP_AUDIT_LOG_DISABLED_EVENTS across both servers does not have an id
rejected as unknown. EVENT_NAMES_TO_IDS deliberately does not: a name like
'query read' exists in both blocks with different ids, and nothing resolves
names at runtime.
Each descriptor tool-call entry now names its service_package, for the same
reason. The AI Data Plane block (0x13000) is recorded as a constant but
left out of SERVICE_PACKAGE_BLOCKS: a block registered without a
classification table would send every call down the fail-closed path.
nithishr
previously approved these changes
Oct 13, 2026
nithishr
left a comment
nithishr
left a comment
Collaborator
There was a problem hiding this comment.
Please check the comments before merging
AayushTyagi1
force-pushed
the
DA-2038-auditability-phase1-synced
branch
from
October 14, 2026 08:10
d25a648 to
8b25914
Compare
This branch has not been deployed
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
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.
Related Issue
Resolves:
Introduce Auditability in MCP
https://docs.google.com/document/d/1svA28Py0TADzPu4Oom1teIJ0KMcVlvp5CxrzeBJVbEA/
What does this change do?
https://docs.google.com/document/d/1svA28Py0TADzPu4Oom1teIJ0KMcVlvp5CxrzeBJVbEA/
Why is this change needed?
https://docs.google.com/document/d/1svA28Py0TADzPu4Oom1teIJ0KMcVlvp5CxrzeBJVbEA/
Evidence of Testing
Automated tests — commands run and results summary:
Environments tested (both are required):
Manual verification — MCP client used (Claude Desktop, Cursor, MCP Inspector, ...) and what was exercised. Screenshots or tool-call transcripts are very helpful:
Compatibility Considerations
Checklist
cb_mcp.corecontracts / managed MCP interfaces (or discussed first)