Repository navigation
Conversation
server/lambda_handler.py duplicated server/adapters/aws_lambda.py with divergent behavior; it was dead code since the deployed handler is server.adapters.aws_lambda.lambda_handler (wired in terraform/aws/main.tf). The root local_server.py duplicated scripts/local_server.py, which is the maintained version (better logging, /mcp routing, OPENCONTEXT_CONFIG support). Docs updated to point at the maintained scripts/local_server.py. Co-Authored-By: Kimi K2.6 via opencode <noreply@ollama.com> (cherry picked from commit ac5d8ac)
scripts/local_server.py serves MCP requests on /mcp (not /), so the FAQ and QUICKSTART curl examples targeting the root path would 404. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 9a311d0)
The warm-start path was as slow as a cold start because lambda_handler called asyncio.run() per request and _run_with_cleanup's finally block shut down the plugin manager after every invocation. The next request then re-initialized the plugin from scratch, including a live HTTP call to the data portal — defeating Lambda container reuse entirely. The per-request shutdown was a workaround for "Event loop is closed" errors with httpx: asyncio.run() creates and closes a fresh loop each call, so httpx clients created on a previous loop broke on warm starts. Replace this with a single module-level event loop created lazily via _get_loop() and reused with loop.run_until_complete(). httpx clients stay bound to a live loop across invocations, so the plugin manager and MCP server globals in server/http_handler.py persist as intended. All existing behavior (OPTIONS handling, base64 bodies, header lowercasing, error responses) is preserved. Co-Authored-By: GLM 5.2 via opencode <noreply@ollama.com> (cherry picked from commit 30ce67b)
…ginConfig, BaseQueryValidator)
The CKAN, Socrata, and ArcGIS plugins currently copy-paste ~60% of their
scaffolding: HTTP client creation/close, the tenacity retry policy,
HTTPStatusError -> RuntimeError translation, the tool-dispatch if/elif
chain with per-arg "is required" checks, Record-N record formatting, and
the SQL/SoQL/WHERE security validators.
These new core/ base classes centralize that shared behavior so the three
plugins can be migrated onto them in a later stage:
- core/config_base.py: BasePluginConfig(pydantic.BaseModel) with
enabled/city_name/timeout fields, extra='forbid', and a reusable
validate_url classmethod that subclasses attach to their URL fields via
field_validator('base_url', 'portal_url')(BasePluginConfig.validate_url).
- core/query_validator.py: BaseQueryValidator with the union of the
existing forbidden keywords, dangerous-pattern regexes, an
ALLOWED_PREFIXES tuple, a validate_query() entry point, an overridable
extra_checks() hook for provider-specific rules, and a standalone
scan_forbidden_keywords() for WHERE-clause-style inputs (ArcGIS).
- core/base_plugin.py: BaseOpenDataPlugin(DataPlugin) that eagerly builds
plugin_config from config_class, tracks HTTP clients in self._clients
for a single shutdown() path, exposes a module-level HTTP_RETRY
decorator, _raise_http_error() that extracts CKAN/Socrata-style error
messages, a ToolHandler-based execute_tool() dispatcher with required-
arg validation and str/ToolResult return wrapping, format_records(),
and build_where_clause(). initialize(), get_tools(), and health_check()
remain abstract for subclasses.
Tests cover URL validation (good/bad/trailing-slash), extra='forbid',
subclass field_validator reuse; query validator allowed/forbidden/length/
prefix/extra_checks override/scan_forbidden_keywords; base plugin
dispatch, unknown tool, required-arg enforcement, exception wrapping,
str-return wrapping, format_records (empty/header/skip_keys/truncation),
build_where_clause (string escaping/IS NULL/numeric/AND join), and
shutdown closing all clients. All 355 tests pass.
Co-Authored-By: GLM 5.2 via opencode <noreply@ollama.com>
(cherry picked from commit d2dec23)
Migrate the CKAN plugin onto the new shared base classes introduced in
refactor/core-base-classes, removing duplicated boilerplate while keeping
all user-facing text and behavior identical.
- config_schema.py: CKANPluginConfig now subclasses BasePluginConfig, reusing
the shared enabled/city_name/timeout fields and BasePluginConfig.validate_url
via field_validator. The duplicated validate_url body is dropped. The CKAN
timeout default is overridden to 120.0 (base default is 30.0) to preserve
the historical 120-second behavior; the bound is widened to le=300 so
existing 120 configs still validate.
- sql_validator.py: SQLValidator now subclasses BaseQueryValidator, reusing
the shared length/forbidden-keyword/prefix/dangerous-pattern checks.
ALLOWED_PREFIXES=('SELECT','WITH'). The CKAN-specific checks (sqlparse
single-statement + statement-type enforcement, and the double-quoted UUID
resource-id validation) move into extra_checks. MAX_SQL_LENGTH is kept as a
backwards-compat alias of MAX_QUERY_LENGTH. tests/test_sql_validator.py
passes unmodified.
- plugin.py: CKANPlugin now subclasses BaseOpenDataPlugin with
config_class=CKANPluginConfig. The ~160-line execute_tool if/elif ladder is
replaced by tool_handlers() returning ToolHandler entries (search_datasets,
get_dataset [dataset_id], query_data [resource_id], get_schema
[resource_id], execute_sql [sql], aggregate_data [resource_id, metrics]).
The HTTP client is created via _create_http_client (keeping
base_url/headers/timeout) so the base class owns shutdown; the plugin's own
shutdown is removed. _call_ckan_api is decorated with HTTP_RETRY from
core.base_plugin instead of a local retry() copy. _parse_ckan_error and the
CKAN-specific 404 resource/dataset parameter hinting are kept in
_call_ckan_api (provider-specific richness not forced into
_raise_http_error). _format_query_results/_format_sql_results now delegate
to self.format_records(...) with their original header text and max_display
values (5 for query results, 10 for SQL results). aggregate_data uses
BaseOpenDataPlugin.build_where_clause for its WHERE construction.
Also fixed while here: aggregate_data's HAVING clause previously hardcoded
'>' for every condition (f'{expr} > {value}'). having now accepts string
values that carry their own operator (e.g. {"count(*)": ">= 5"}); numeric
values keep '>' as the documented default for backward compatibility. The
aggregate_data tool description documents this.
- tests/test_ckan_plugin.py: updated test_plugin_shutdown_closes_client to
assert the base class clears the tracked client list (plugin._clients == [])
instead of the old plugin.client is None, since shutdown is now owned by
the base. All other behavioral assertions are unchanged; the rest of the
suite (mocking httpx.AsyncClient, which _create_http_client still calls)
passes unmodified.
Co-Authored-By: GLM 5.2 via opencode <noreply@ollama.com>
(cherry picked from commit 2b166e6)
Port this hardening (adapted from upstream thealphacubicle/OpenContext security update #37) so aggregate_data validates all user-supplied SQL fragments before assembling the query, preventing SQL injection through field names, metric aliases/expressions, or order_by. - Add module-level _SAFE_IDENTIFIER = r'^[a-zA-Z_][a-zA-Z0-9_]{0,63}$' and _SAFE_METRIC_EXPR (count(*) | sum/avg/min/max/stddev/variance(field), case-insensitive) compiled patterns with _validate_identifier(name) and _validate_metric_expr(expr) helpers that raise ValueError on mismatch. - In aggregate_data, validate all group_by fields, metric aliases and expressions, filter field names, having expression identifiers, and order_by (stripping a leading '-' for descending order) BEFORE building SQL. Validation failures return {"error": True, "message": ...} so the dict-return contract is preserved. - Add tests/test_ckan_plugin.py::TestAggregateDataHardening covering rejected malicious group_by, metric alias, metric expression, filter field, having expression, and order_by, plus a valid-aggregate and a leading-dash order_by happy path. Ported from thealphacubicle/OpenContext (Feature/security update #37). Co-Authored-By: GLM 5.2 via opencode <noreply@ollama.com> (cherry picked from commit 6b31bb8)
Migrate Socrata plugin onto BaseOpenDataPlugin / BasePluginConfig / BaseQueryValidator following the CKAN reference pattern: - config_schema.py: SocrataPluginConfig now inherits BasePluginConfig; reuse BasePluginConfig.validate_url via field_validator; drop duplicated enabled, city_name, timeout, and the copy-pasted validate_url. - soql_validator.py: SoQLValidator now inherits BaseQueryValidator; move the extra semicolon multiple-statement check into extra_checks; keep public API validate_query(soql) -> (bool, Optional[str]). - plugin.py: SocrataPlugin now inherits BaseOpenDataPlugin; config_class set; replace execute_tool if/elif ladder with tool_handlers(); create both HTTP clients via _create_http_client (tracked for base-class shutdown); replace copy-pasted @Retry blocks with @HTTP_RETRY; use self._raise_http_error for HTTPStatusError translation; replace _format_query_results/_format_sql_results bodies with self.format_records(...); use BaseOpenDataPlugin.build_where_clause in query_data filter compilation; remove redundant app_token emptiness re-check in initialize(). Bug fixes folded in: - _format_categories: fix duplicated fallback key cat.get("count", cat.get("count", "")) to cat.get("count", cat.get("value", "")). - _list_categories: add max-pages guard (20 pages / 10000 results) to the unbounded pagination while-loop, with a warning log when hit. Tests updated minimally: shutdown assertions now check is_initialized only (base class manages client lifecycle via _clients list). Co-Authored-By: Kimi K2.6 via opencode <noreply@ollama.com> (cherry picked from commit 1efa1fc)
…ith plugin contract Migrates the ArcGIS plugin onto the shared base classes introduced in d2dec23, following the patterns established by the CKAN and Socrata migrations. Moved to the base classes: - config_schema.py: ArcGISPluginConfig now subclasses BasePluginConfig. Drops the duplicated enabled/city_name/timeout fields and the copy- pasted validate_url; reuses the base URL validator via field_validator (as CKAN/Socrata do). Preserves the 120s timeout default by overriding the timeout field default to 120.0 (base default is 30.0). portal_url default and token field retained. - where_validator.py: WhereValidator now subclasses BaseQueryValidator and reuses scan_forbidden_keywords for its forbidden-keyword scan. Public API validate(where) -> str is preserved (returns '1=1' for empty, raises ValueError on forbidden keywords). As a result, ArcGIS gains the GRANT/REVOKE/DECLARE/SET keywords it was previously missing versus the other plugins (its old FORBIDDEN_KEYWORDS list omitted them). The error message format changes slightly ('Forbidden keyword detected in WHERE clause: Forbidden keyword: X'). - plugin.py: ArcGISPlugin now subclasses BaseOpenDataPlugin. Config is validated eagerly in __init__ (base behavior) instead of lazily in initialize(); get_tools() uses self.plugin_config.city_name directly (the 'Unknown' fallback is dropped). Both HTTP clients are created via _create_http_client so the base class tracks them for shutdown; the plugin's own shutdown() is removed. The execute_tool if/elif ladder is replaced with tool_handlers(). health_check now relies on raise_for_status() (via _call_hub_api) instead of checking status_code == 200, matching the other plugins. _raise_http_error is used for HTTP status errors on both API paths. An _call_hub_api helper (decorated with @HTTP_RETRY) centralizes hub-client GETs, and _call_feature_service (also @HTTP_RETRY) handles feature-service GETs. ArcGIS was previously the only plugin with NO retry on its API calls; both call paths now retry transient failures (ConnectError, ReadTimeout, etc.) up to 3 attempts with exponential backoff, while HTTPStatusError and RuntimeError are not retried. Contract fixes (DataPlugin interface alignment): a. search_datasets tool input parameter renames from 'q' to 'query' to match CKAN/Socrata and the DataPlugin interface; the tool schema description is updated accordingly. b. Adds a get_schema tool + method: fetches the Feature Service layer metadata (GET {service_url}/0?f=json after _ensure_layer_url) and formats the 'fields' list (name, type, alias) like the other plugins' get_schema. Required arg is dataset_id; the service URL is resolved via get_dataset (two-hop, like query_data). If the dataset has no service URL, a clear ValueError is raised. c. DataPlugin.query_data(resource_id, filters, limit) previously repurposed the filters dict to carry where/out_fields. Restructured: an internal async _query_features(dataset_id, where, out_fields, limit) carries the tool-path behavior (tool args unchanged: where, out_fields, limit), and query_data(resource_id, filters, limit) now implements the interface contract — compiling field:value filters into a WHERE clause via BaseOpenDataPlugin.build_where_clause (validating each field identifier with WhereValidator.scan_forbidden _keywords first), defaulting to '1=1' when no filters are supplied. All other user-facing text (search/dataset/query/aggregation formats) is kept as-is. Tests: tests/test_arcgis_plugin.py is rewritten for the new structure — search_datasets param rename (query vs q), get_schema tests (fields, layer-index append, no-service-URL error), query_data contract tests (filters->WHERE compilation, None->IS NULL, forbidden field rejection, 1=1 default), retry behavior on transient errors (ConnectError retried, HTTPStatusError not retried), health_check uses raise_for_status, eager config validation in __init__, two client creation via _create_http_client, base shutdown closes tracked clients. WhereValidator tests cover the newly-gained GRANT/REVOKE/ DECLARE/SET keywords. 62 tests (up from 22), all passing. Co-Authored-By: GLM 5.2 via opencode <noreply@ollama.com> (cherry picked from commit a8f22c9)
Ported from thealphacubicle/OpenContext (Feature/security update #37). Prevents a crafted dataset record from steering Feature Service queries to arbitrary hosts. Adds a static _validate_feature_url(service_url, portal_url) helper that parses the resolved Feature Service URL and requires the scheme to be http/https and the host to either end with '.arcgis.com' or equal the configured portal host (case-insensitive), raising ValueError otherwise. The guard is invoked on the resolved service_url in both Feature-Service query paths (get_schema and _query_features) before any HTTP request is made. Unit tests cover allowed hosts (arcgis.com subdomains, exact portal host match, case-insensitivity) and rejected hosts (arbitrary domains, localhost, 169.254.169.254 metadata IP, non-http schemes, missing hostname, lookalike hosts). Integration tests verify the guard fires in both get_schema and _query_features. Co-Authored-By: GLM 5.2 via opencode <noreply@ollama.com> (cherry picked from commit 4b9dddb)
…nManager 1. custom_plugins/template/plugin_template.py - Rewrite to inherit from BaseOpenDataPlugin (was raw MCPPlugin) - Define config class subclassing BasePluginConfig, set config_class attr - initialize() uses _create_http_client for tracked HTTP lifecycle - Demonstrate tool_handlers() with required_args via ToolHandler - Provide get_tools(), health_check(), and DataPlugin stub methods - Keep heavily commented as a teaching template 2. server/http_handler.py - Move import-time config/logging load into _configure_logging_from_config() - Call it from _initialize_server() before plugin loading - Module-level default configure_json_logging(level='INFO') keeps imports side-effect-free - Preserve OPENCONTEXT_CONFIG env var behavior 3. scripts/local_server.py - Move import-time config load and logging setup into _load_local_config() - Run both in init_server() instead of at module import - Replace global with updated at init time 4. core/plugin_manager.py - discover_plugins() now returns (name, path, source_package) tuples - _load_plugin_class() receives source_package directly instead of substring-matching paths - Store ToolDefinition in self.tools during registration; value becomes (plugin_name, ToolDefinition) - get_all_tools() projects directly from self.tools, removing re-call to plugin.get_tools() - execute_tool() uses tool_def.name to dispatch, keeping public shapes identical 5. core/validators.py - Remove dead defensive check after validate_plugin_count() - Leave multiple-plugins error text untouched 6. docs/CUSTOM_PLUGINS.md - Update plugin structure bullet to mention BaseOpenDataPlugin - Add tip recommending BaseOpenDataPlugin as the starting point 7. tests/test_plugin_manager.py - Update discovery assertion to expect 3-tuple (name, path, source_package) - Update tool registration assertions to inspect ToolDefinition.name - Update execute_tool manual injection to use ToolDefinition instead of bare string - Update _load_plugin_class call sites for new signature Co-Authored-By: Kimi K2.6 via opencode <noreply@ollama.com> (cherry picked from commit b59c05c)
…ning
Code review of the hardening commit found four behavior regressions
against main, all fixed here:
- order_by "field DESC" was rejected and "-field" produced invalid SQL
("ORDER BY -field"); order_by now accepts "field", "-field", and
"field ASC|DESC", compiling to proper ORDER BY ... [ASC|DESC].
- count(field) and count(distinct field) were banned while sum(field)
was allowed; the metric whitelist now permits them.
- HAVING keys naming a declared metric alias errored; aliases are now
substituted with their expression (PostgreSQL does not allow SELECT
aliases in HAVING).
- String HAVING values were interpolated verbatim ("HAVING count(*) 5"
for "5"); values must now be a number with an optional comparison
operator, and bare numbers default to ">".
Also enforce the search_datasets required 'query' argument at dispatch
(the schema declared it required but nothing checked it).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 58f7956)
Code review found the helper trusted callers to validate field names (only its docstring warned about it), and the three plugins guarded three different ways — Socrata's query_data not at all. Enforce a safe identifier pattern in the helper itself so every caller, including third-party plugins built from the template, is safe by construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 113c15f)
Code review of the migration + SSRF commits found four behavior
regressions against main, all fixed here:
- The SSRF allowlist (*.arcgis.com or portal host only) broke Hub
datasets whose Feature Services are self-hosted on city domains,
common in Hub catalogs. New trusted_service_hosts config lists extra
hosts (exact or subdomain match) without reopening arbitrary-host SSRF.
- WhereValidator scanned quoted string literals, rejecting legitimate
values like status = 'SET' or call_type = 'Initial Call'. Literals
are now stripped before the keyword scan; the double-prefixed error
message is also fixed.
- search_datasets declared 'query' required but never enforced it, so
old-schema {"q": ...} calls silently returned an unfiltered catalog
dump as success. The required arg is now enforced at dispatch, and
get_aggregations' remaining 'q' parameter is renamed to 'query' for
consistency.
- _format_query_results hand-rolled an uncapped record loop; it now
uses the shared format_records helper (10-record display cap).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 7f177f5)
The tool schema declared 'query' required but dispatch never checked it, so calls without it silently searched with an empty query. Found in code review; same gap fixed in the CKAN and ArcGIS plugins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit c7f5b42)
- Matrix the test suite across Python 3.11 and 3.12 - Add Terraform validate job (fmt -check + validate for aws and bootstrap stacks) - Add Dependabot config for pip, gomod, github-actions, and terraform - Ignore Claude Code local files (CLAUDE.md, .claude/) - terraform fmt fixes for staging.tfvars and variables.tf Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 0b5222b)
…c reuse The field_validator(...)(BasePluginConfig.validate_url) reuse pattern broke on Python 3.11/3.12 (CI) while passing on 3.14: the bound classmethod received pydantic's (value, info) pair positionally, so 'cls' swallowed the value and urlparse got a ValidationInfo. A single-argument staticmethod binds identically on every version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit ad62d8b)
- CUSTOM_PLUGINS.md: new "Decoupled Base Layer" section (BaseOpenDataPlugin, ToolHandler dispatch, BasePluginConfig, BaseQueryValidator) with examples - ARCHITECTURE.md: updated component tree and shared base layer overview - BUILT_IN_PLUGINS.md: ArcGIS plugin section; CKAN execute_sql and aggregate_data (metrics/having/order_by semantics) - config-example.yaml: document trusted_service_hosts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 14921aa)
The workflow merges main into a `develop` branch on every push to main, but no `develop` branch exists on this fork or upstream, so the job has failed on every push since it was added. Development lands on main via PRs, so the sync has nothing to do; remove it rather than create an unused branch. Claude-Session: https://claude.ai/code/session_01TXm8YpvmTqNHgpRuoxtpoX Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit fcd1549)
Adds a provider plugin for Opendatasoft-based open data portals, built on
the shared decoupled base layer: OpendatasoftPlugin subclasses
BaseOpenDataPlugin (HTTP client lifecycle via _create_http_client, HTTP_RETRY,
_raise_http_error, ToolHandler dispatch with required-arg enforcement,
format_records/build_where_clause), and OpendatasoftPluginConfig subclasses
BasePluginConfig, reusing BasePluginConfig.validate_url as a staticmethod via
field_validator("base_url", "portal_url").
Toolset (6 tools): search_datasets, get_dataset, get_schema, query_data,
aggregate_data, list_categories — backed by the Explore v2.1 endpoints
/catalog/datasets, /catalog/datasets/{id}, /catalog/datasets/{id}/records and
/catalog/facets?facet=theme.
ODSQL validation: ODSQLValidator subclasses BaseQueryValidator and validates
where/select/order_by fragments by stripping single- and double-quoted string
literals before the shared forbidden-keyword scan, so keywords that appear
inside legitimate data values are not rejected. aggregate_data additionally
whitelists group_by fields and metric aliases with a safe-identifier regex and
metric expressions with a safe-aggregate regex covering count(*),
count(field), count(distinct field) and sum/avg/min/max(field), and accepts
the "field" | "-field" | "field ASC|DESC" order_by grammar (metric aliases
allowed, since ODSQL permits ordering by select aliases).
Long Beach (https://data.longbeach.gov) is the reference portal; all tests
mock HTTP and make no live network calls.
Co-Authored-By: Claude Opus <noreply@anthropic.com>
(cherry picked from commit 6d1fbcd)
Adds an "Opendatasoft Plugin" section to docs/BUILT_IN_PLUGINS.md covering the configuration block, the six tools, ODSQL notes and validation behavior, the Explore v2.1 endpoints used, and Long Beach examples. Adds a commented, disabled opendatasoft block to config-example.yaml alongside the other built-in plugin examples. Co-Authored-By: Claude Opus <noreply@anthropic.com> (cherry picked from commit 5da76cc)
Live testing against data.longbeach.gov showed the Explore API repeats a group_by-less aggregate once per underlying record (100 identical rows for count(*)). Request limit=1 when no group_by is given — the single row is the whole answer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit b1e7888)
Seven confirmed findings fixed (the eighth, validator duplication across
plugins, is deferred to a core-level refactor):
- dataset_id was interpolated raw into the request path; a crafted id
('realid/exports/json?', '../facets') could redirect requests to other
endpoints. Ids are now validated as URL-slug identifiers at all three
call sites.
- The DataPlugin query_data path escaped string filters by doubling
single quotes (SQL convention) — an ODSQL syntax error for values like
"Val-d'Or". New _build_odsql_where emits double-quoted literals with
backslash escapes, ODSQL's convention.
- strip_literals blanked single-quoted spans before double-quoted ones,
so an apostrophe inside a double-quoted literal could hide structural
keywords from the forbidden-keyword scan. Both literal kinds are now
matched in one left-to-right pass.
- Limits are clamped to the API's 1..100 range on every path (query,
search, aggregate); previously limit=0/-1/500 produced empty results
or HTTP 400s.
- aggregate_data now coerces a bare-string group_by into a one-element
list instead of iterating it character by character.
- Query/aggregate formatters display every fetched record instead of
hardcoding a 10-row cap that discarded up to 90% of the transfer.
- count() with no argument now fails validation with a clear message
instead of reaching the API as an ODSQL syntax error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 50fabac)
All existing security work guards the outbound direction (LLM -> portal: SQL/SoQL validators, identifier whitelists, ArcGIS SSRF allow-list). Nothing guarded the inbound direction: every string a portal returned -- dataset descriptions, schema labels, error bodies, and record values -- was f-string-concatenated verbatim into tool results. Public datasets (311, permits, comments) contain text submitted by the public, so an attacker can plant instructions without compromising the portal, and hosts routinely pair this connector with tools that can act (email, files, calendar). New core/portal_content.py, applied centrally by BaseOpenDataPlugin: - Untrusted-data boundary: every successful text result is wrapped in a preamble + <<<BEGIN/END PORTAL DATA>>> markers; connector guidance moves to ToolHandler(guidance=...) and is emitted after the closing marker so instruction-shaped text never sits inside the data region. - Normalization: strip control/zero-width/bidi/tag/private-use code points, collapse newlines in single-line fields, explicit truncation markers, defang literal boundary markers; per-value, per-line, per-response caps. - Structure-forgery prevention: format_records keys are single-line and multi-line values/descriptions indent continuation lines so a value cannot fake a "Record N:" header or a connector hint at column 0. - safe_id + per-plugin id_pattern: IDs are only interpolated into Portal: links and hints if they match the provider's ID shape; links are built from config, never echoed from the portal. - ArcGIS _display_url: portal-supplied URLs shown only if the host passes the same allow-list that gates fetching. - Error bodies: _raise_http_error and JSON-RPC error.data cap/flatten portal text and label it "portal said:". - Heuristic injection detection: never blocks; prepends a WARNING line and logs a "Possible prompt injection markers" entry with the tool name for operator visibility. - MCP tool annotations: readOnlyHint/openWorldHint on every tool. Plugins (CKAN, Socrata, ArcGIS, Opendatasoft) route metadata through portal_line/portal_block/safe_id. docs/SECURITY.md documents the threat model and what the connector can and cannot guarantee. 37 new tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U7kJqaZ5pxpAKbZBr1biBs (cherry picked from commit ee59322)
Socrata occasionally migrates a portal's domain (e.g. data.sfgov.org -> data.sf.gov) and 301s every path on the old one. httpx.AsyncClient defaults to not following redirects, so a portal_url that lags a rename got back a raw 301 with an HTML body: raise_for_status() didn't catch it (only 4xx/5xx), and response.json() failed trying to parse the redirect page. That broke get_schema/get_dataset/query_dataset/ execute_sql while search_datasets kept working, since the Discovery API treats old/new Socrata domains as aliases for search. Hit this live: data.sfgov.org started 301-redirecting to data.sf.gov, and every SODA3 tool failed until the deployment's portal_url was updated and this fix landed. Adds follow_redirects=True to the SODA client (the Discovery client is unaffected — api.us.socrata.com is stable infra, not a per-portal domain) plus a test asserting it's set. Claude-Session: https://claude.ai/code/session_01HuLvMhEAJgh6M1Rih5gWpD Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> (cherry picked from commit 1f3b400)
…s everywhere (#20) Follow-up to #19. #19 made the Socrata SODA client follow redirects so a renamed portal domain (data.sfgov.org -> data.sf.gov) keeps working, but httpx only strips Authorization on cross-origin redirects — not custom headers such as Socrata's X-App-Token. A lapsed portal domain can be re-registered by someone else, who would then receive the token (and, for the other providers, any api key) from every deployment whose portal_url lags the rename. core/base_plugin.py: _create_http_client gains protect_headers= and trusted_hosts=. When set, it follows redirects and attaches a request event hook that drops those header names on any hop (initial or redirect) whose host is not the configured portal/base host, a subdomain of it, or a listed extra host. Shared _host_is_trusted / _trusted_request_hosts helpers. All four plugins opt in with their credential header: - Socrata SODA client: X-App-Token (replaces the bare follow_redirects=True) - CKAN, Opendatasoft: Authorization (also newly follow redirects) - ArcGIS hub + feature clients: Authorization, trusting *.arcgis.com and trusted_service_hosts (where feature services live) A legitimate rename is indistinguishable from a hijack, so the credential is not forwarded to the new host; the request still follows through unauthenticated (Socrata's app token is only a rate-limit key; public data on the others still resolves). Operators should update portal_url to the new domain. Tests: base hook coverage (same/subdomain kept, untrusted dropped + logged, rename drops, extra trusted host retained, follow_redirects forced) and a Socrata assertion that the SODA client opts in. docs/SECURITY.md documents the scoping. 624 tests pass; no new ruff findings vs main. Claude-Session: https://claude.ai/code/session_01U7kJqaZ5pxpAKbZBr1biBs Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 260fbc0)
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.
Third of the four-PR stack; stacked on #3 (review the last 3 commits).
What changes
core/portal_content.py, applied in all four plugins). Text that comes from a portal (dataset titles, descriptions, tags, organization names) is untrusted input to the model. Tool output now frames it as such, normalizes control characters and whitespace, hands back identifiers throughsafe_id, gates URLs throughdisplay_portal_urlso only the configured portal host is echoed, and warns when injection-marker patterns appear. Tools carry MCPreadOnlyHint/openWorldHintannotations.get_schema/get_dataset/query_dataset, not only for search._create_http_client(protect_headers=..., trusted_hosts=...)). httpx stripsAuthorizationon cross-origin redirects but not custom headers such as Socrata'sX-App-Token. A request hook now drops the credential header on any hop whose host is not the configured portal host, a subdomain of it, or a listed trusted host (ArcGIS trusts*.arcgis.comandtrusted_service_hosts). A lapsed portal domain can be re-registered by someone else, so the token is never forwarded to it; the request still follows through unauthenticated.docs/SECURITY.mddocuments the threat model and each defense.Verification
tests/test_portal_content.py,tests/test_base_plugin.py(redirect scoping), and the Socrata tests.🤖 Generated with Claude Code
https://claude.ai/code/session_01TXm8YpvmTqNHgpRuoxtpoX