From c89f724585fca4961ded3f60d39bdb8bfc49a4c0 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 7 Jun 2026 15:47:57 +0200 Subject: [PATCH] refactor(usage): close out round-5 nits on PR #871 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-blocking follow-ups from the merge-ready review: - semantic.py: bound the doc_types copied into embeddings_queries metadata to _USAGE_METADATA_MAX_DOC_TYPES (16). doc_types is caller-supplied with no max_length on the tool signature; capping the stored copy keeps one JSONB row from ballooning (not a billing/injection risk — CP ignores metadata, binds are parameterized). - migration: note that `metric` is intentionally unconstrained Text and that adding a third metric requires keeping the CP-side catalog in sync, else the rollup silently ignores the new rows. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../20260607_1200_007_add_usage_events.py | 6 +++++- nextcloud_mcp_server/server/semantic.py | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/nextcloud_mcp_server/alembic/versions/20260607_1200_007_add_usage_events.py b/nextcloud_mcp_server/alembic/versions/20260607_1200_007_add_usage_events.py index dc61317a..6ce5bf32 100644 --- a/nextcloud_mcp_server/alembic/versions/20260607_1200_007_add_usage_events.py +++ b/nextcloud_mcp_server/alembic/versions/20260607_1200_007_add_usage_events.py @@ -55,7 +55,11 @@ def upgrade() -> None: postgresql.TIMESTAMP(timezone=True) if is_pg else sa.TIMESTAMP(), nullable=False, ), - # Catalog metric: 'embeddings_queries' or 'pages_chunks'. + # Catalog metric: 'embeddings_queries' or 'pages_chunks'. Deliberately + # an unconstrained Text (no CHECK/enum) — the metric catalog lives in + # control-plane config, not the app-DB schema. If a third metric is + # ever added, the CP-side catalog must learn it too, or its rollup will + # silently ignore the new rows; keep the two in sync. sa.Column("metric", sa.Text(), nullable=False), sa.Column("value", sa.BigInteger(), nullable=False), # Rawest unit per request (provider, model, tokens, doc_type, ...). diff --git a/nextcloud_mcp_server/server/semantic.py b/nextcloud_mcp_server/server/semantic.py index f8d36221..ec93a851 100644 --- a/nextcloud_mcp_server/server/semantic.py +++ b/nextcloud_mcp_server/server/semantic.py @@ -46,6 +46,15 @@ from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client logger = logging.getLogger(__name__) +# Cap how many doc_types we copy into a usage-metering metadata row. doc_types +# is caller-supplied and (unlike path_prefixes) has no max_length on the tool +# signature, so an adversarial caller could pass a huge list. The CP rollup +# ignores metadata for billing (GROUP BY day, metric) and the value is bound +# parameterized, so this is not a billing/injection risk — the cap just keeps +# a single JSONB row from ballooning. 16 is generous headroom over the handful +# of real indexed doc types. +_USAGE_METADATA_MAX_DOC_TYPES = 16 + def configure_semantic_tools(mcp: FastMCP): """Configure semantic search tools for MCP server.""" @@ -538,7 +547,12 @@ def configure_semantic_tools(mcp: FastMCP): metadata={ "user_id": username, "fusion": fusion, - "doc_types": doc_types, + # Bounded copy — see _USAGE_METADATA_MAX_DOC_TYPES. + "doc_types": ( + doc_types[:_USAGE_METADATA_MAX_DOC_TYPES] + if doc_types + else doc_types + ), }, # The outer guard already confirmed the flag, so pass # enabled=True directly — the store then skips a second