Repository navigation
fix(arcgis, socrata): Hub-referenced service hosts, layer resolution, f=json, schema fallback, optional app token - #6
Open
sgarcese wants to merge 29 commits into
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)
…#14) * feat(ckan): surface catalog metadata and add list_datasets + get_catalog_stats A Boston eval comparing an agent with and without OpenContext flagged the connector's top gap: get_dataset summarized package_show into prose and dropped metadata_modified, license, organization, tags, and every resource's url/created/last_modified (the resource URL alone would have dated the year-series resources the agent had to leave null), and there was no way to count the catalog, so catalog-wide statistics were asserted instead of counted. core/base_plugin.py (shared by all plugins): - short_date (ISO / epoch s / epoch ms -> YYYY-MM-DD), human_size, format_search_header (catalog-wide total + "showing a-b"), and display_portal_url: a portal-supplied URL is echoed only when its host is the portal/API host or a subdomain (or an extra trusted host); otherwise only "(external: hostname)" is shown. Generalizes ArcGIS _display_url. plugins/ckan/plugin.py: - get_dataset now prints organization (title + slug), license (title + id), created/modified, tags, groups, and per resource: created, modified (last_modified or metadata_modified), size, DataStore flag, gated download URL and description. New max_resources arg (default 50, max 500) with an explicit "... and N more" line. - search_datasets header uses package_search's count; rows show organization, modified date, resource count + formats, tag count. - New list_datasets: exact-match filters (organization/tag/format/license/ group), sort enum (default metadata_modified desc), limit/offset, total count. Filters are whitelisted and values quoted as escaped Solr phrases (_build_fq), so model input cannot alter the fq query. fl is deliberately not used (it collapses organization and drops resources). - New get_catalog_stats: package_search rows=0 + facet.field for organization/tags/res_format/license_id/groups, optional query/filters, values sorted by count; falls back to legacy `facets` on older CKAN. - Guidance strings point the model between the three catalog tools. Tests: 22 new (Solr escaping/whitelist, enriched formatters incl. external and non-http resource URLs, max_resources clamp/truncation, count header, list_datasets request shape and invalid sort, catalog stats request shape, sorting, legacy fallback, empty facets, unknown facet). Docs updated (BUILT_IN_PLUGINS, ARCHITECTURE incl. previously missing aggregate_data, SECURITY). Verified live against data.boston.gov: 235 datasets, facet buckets, boston-311-org listing sorted by modified, 311 dataset with 21 dated resources. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U7kJqaZ5pxpAKbZBr1biBs * style: datetime.UTC, ClassVar test fixtures, drop unused noqa Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U7kJqaZ5pxpAKbZBr1biBs * style: ClassVar on remaining test fixtures Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U7kJqaZ5pxpAKbZBr1biBs * style: hoist CKAN helper imports to module top (CI ruff E402) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U7kJqaZ5pxpAKbZBr1biBs --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 384b3d0)
…ta, ODS, ArcGIS (#16) Companion to the CKAN catalog work (#14): the same shape of loss existed in the other three providers — none passed the search total to its formatter (Discovery resultSetSize / ODS total_count / Hub numberMatched), and each get_dataset dropped dates, license, publisher/attribution and counts the API had already returned. Socrata: _discovery_search returns the envelope; search header uses resultSetSize and rows show modified date, column count, downloads, source. get_dataset adds source/attribution (link host-gated), license name + id (terms link host-gated) instead of a stringified dict, created/published/ metadata-modified/rows-updated dates, rows/columns/downloads/views/ provenance; empty values are omitted instead of "N/A". Opendatasoft: _catalog_search returns the envelope; header uses total_count (the pattern _tool_query_data already used) and rows show publisher, modified, records. get_dataset adds publisher, license (+ gated URL), attribution, data/metadata processed dates, field count, references. ArcGIS: _search_hub returns results + numberMatched; rows show owner, created/modified, record count. get_dataset adds organization, last-edit date, size, record count, categories, type keywords, access information; empty fields are omitted. _display_url now delegates to the shared display_portal_url with *.arcgis.com + trusted_service_hosts, so untrusted hosts render as "(external: host)" like every other provider. Tests: base helper coverage (short_date, human_size, display_portal_url, format_search_header) plus enrichment tests per provider. Docs updated. Claude-Session: https://claude.ai/code/session_01U7kJqaZ5pxpAKbZBr1biBs Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 0ae1dcb)
Socrata's developer console issues two different credential types:
a single-string App Token (Developer Settings -> App Tokens), and an
API Key pair (Key ID + Key Secret, Developer Settings -> API Keys)
meant for HTTP Basic Auth on authenticated requests. This plugin only
implements the former, sending app_token bare as X-App-Token.
Pasting an API Key's Key ID in as app_token fails silently for some
tools and not others: search_datasets/get_dataset keep working
(catalog/metadata calls tolerate it), but query_dataset fails with
"Invalid app_token specified" (403) on /resource/{id}.json — the
actual SODA3 data-query endpoint. Confirmed against data.ny.gov,
data.cityofnewyork.us, data.lacity.org, and data.sfgov.org while
deploying new portals.
Updates the config schema field description/validation error and
BUILT_IN_PLUGINS.md to steer setup toward the right credential type.
No auth code changes needed — a bare App Token is sufficient since
Socrata open-data portals are all public.
Claude-Session: https://claude.ai/code/session_01HuLvMhEAJgh6M1Rih5gWpD
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
(cherry picked from commit e1ff187)
… f=json, schema fallback, optional app token
Findings from a 51-jurisdiction open-data audit run through the connector,
plus regressions found against HUD's ArcGIS Hub and data.cdc.gov:
- ArcGIS `auto_trust_hub_services` (default true): a Hub-referenced Feature
Service URL is accepted when it is https, on a public DNS name, and has an
ArcGIS REST service path; IP literals, single-label/.internal/.local
names, http URLs and non-service paths are refused. *.arcgis.com, the
portal host and `trusted_service_hosts` win as before; the bearer token is
never sent to auto-trusted hosts. Refusals start with
`untrusted_service_host: '<host>'` and name the config key.
- get_schema passed `params={}` with `?f=json` in the URL; httpx replaces the
URL query with params, so ArcGIS answered HTML on every dataset. `f=json`
now travels as a param, and a one-row /query fallback derives the schema
when the metadata endpoint is genuinely unusable.
- `_resolve_layer_url` reads the service description once and uses the
first layer id (then table, then 0); services whose only layer is not 0
(HUD Low-Mod Income by Tract = 4, Opportunity Zones = 13) were unreachable.
- Socrata `app_token` is optional; blank means no X-App-Token header
(data.cdc.gov rejects an invalid token but serves untokened requests).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXm8YpvmTqNHgpRuoxtpoX
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.
Fifth of the stack; stacked on #5 (review the last commit). Addresses ArcGIS and Socrata tool failures found by a 51-jurisdiction open-data audit run through the connector, and regressions found against HUD's ArcGIS Hub and data.cdc.gov.
Problems observed
maps2.dcgis.dc.gov,gis.charlottenc.gov,gis.indy.gov). The SSRF guard refused any host not pre-listed intrusted_service_hosts, soquery_data/get_schemafailed for those cities. Denver was the only jurisdiction of 51 with no row-level evidence, purely from this.get_schemafailed on every dataset on a HUD deployment: the plugin built{url}/0?f=jsonand passedparams={}, and httpx replaces the URL's query string withparams, so ArcGIS answered HTML.Changes
auto_trust_hub_services(new ArcGIS config, default true). A Hub-referenced service URL is accepted when it is https, on a public DNS name, and has an ArcGIS REST service path (/rest/services/.../FeatureServer|MapServer[/layer]). IP literals (cloud metadata endpoints), single-label names,.internal/.localnames, http URLs, and non-service paths are refused.*.arcgis.com, the portal host, andtrusted_service_hostsare trusted as before and win over the auto-trust rules. The bearer token is never sent to an auto-trusted host, sinceprotect_headers(PR security: guardrails for untrusted portal content; scope credentials across redirects #4) scopes it to the portal,*.arcgis.com, and the allow-list. Operators wanting a strict allow-list set it to false.untrusted_service_host: '<host>'and name the config key.f=jsontravels as a query parameter on the layer metadata call._resolve_layer_urlreads the service description once, uses the first layer id (then first table, then 0), caches per service, and keeps an explicit/N. Used byget_schemaandquery_data.get_schemaderives the field list from a one-row/query.app_tokenis optional; blank means noX-App-Tokenheader.config-example.yaml, andSECURITY.mdupdated.Verification
f=jsonparam, Socrata no-token header behavior).main(the three remaining are in this repo'stests/test_auth.py).f=json, and the optional token were also verified live against HUD and data.cdc.gov.🤖 Generated with Claude Code
https://claude.ai/code/session_01TXm8YpvmTqNHgpRuoxtpoX