Repository navigation
Odoo new-contact: taught and read back, refused under clinical-write; shipped SQL judged by its effect - #1624
Merged
Merged
Conversation
A registry of compiled skills. pull verifies a keyless Sigstore attestation from the registry release workflow, naming the exact index and archives; refuses an expired index and one older than the newest seen from the same attesting identity at any address; and marks the pulled bundle so run, replay and emit refuse it until skill qualify passes on the user's own instance.
… judged by its effect The registry's first non-healthcare candidate, on a synthetic Odoo 18 fixture. Qualification under clinical-write refuses, as it should, on short-label clicks and an icon-only save button, so no entry ships. A shipped SQL read-back now runs on a driver-level read-only session, and must be SELECT <plain columns> FROM <tables> with inputs bound only after WHERE, so a select list that echoes the inputs can't make a wrong save verify.
… 2.x in the test The check job installed the engine without hashes; it now uses locked_install.py like every other job. The shipped Sigstore trusted root is added to the reviewed public artifact inventory. The emitted MCP server test stands in for FastMCP, which mcp 2.x renamed, as test_emit does.
# Conflicts: # packages/flow/openadapt_flow/skills.py # packages/flow/registry/verify-requirements.txt # packages/flow/scripts/build_registry_index.py
abrichr
marked this pull request as ready for review
October 29, 2026 01:53
abrichr
enabled auto-merge (squash)
October 29, 2026 01:53
abrichr
added a commit
that referenced
this pull request
Oct 29, 2026
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.
Replaces #1605 with one squashed commit on top of #1623 (which replaced #1592), so the secret scan no longer reads the old registry commits. Retarget to main once #1623 merges.
Review history:
Addressed:
PRAGMA query_onlyfor sqlite3,set_session(readonly=True)for psycopg2,read_onlyfor psycopg,SET SESSION TRANSACTION READ ONLYfor the MySQL drivers). A shipped contract may name only those drivers, sopymssqlis dropped.SELECT <plain columns> FROM <tables>, with::text/::varcharandASallowed. Inputs may be bound only afterWHERE, and theFROMpart may not call functions. The review's four queries are now refusals intest_shipped_sql.py, along with a function inFROMand a query with noFROM.qualify.logand the fixture default no longer name a local scratch path. The README drops the VM notes and says onlyqualification/is committed.Not done here: narrowing the Odoo match key with
create_date >= %(started_at)s. That's needed before this becomes a registry entry, and this PR adds none.Stacked on #1592. This is the skills registry's first non-healthcare candidate: create a contact in Odoo 18 Community (LGPL-3.0). It runs on a local test copy holding synthetic data, with Odoo's images pulled by digest at run time.
What ran
benchmark/odoo_contactsbrings up Odoo 18.0 and Postgres 16 on loopback, with a dedicated user and a read-only record-check role. It needed two fixes on its first run:demo.py) chooses Individual, fills name, street, city, ZIP, phone and email, and saves. The contact is saved, and the record check reads all six fields back. The compiled program has 14 steps and 6 parameters (results/2026-10-28-click-teach/).clinical-writerefused before any case, as it should:So no registry entry yet. Per #1598 the next step is the icon-button identity in #1581, not a page heuristic here. The details are in
benchmark/odoo_contacts/README.md.Shipped SQL is one read-only SELECT
As the coordinator decided, a public bundle may ship a minimal SQL record check: the typed fields, keyed by the identity field, on the app's documented schema.
skills.shipped_sql_problemsrequires a single parameterized, read-only SELECT. It refuses:${}text;INTO,FOR UPDATE,pg_*,lo_*anddblink;%(name)s, and any binding that isn't{param: ...}.skill qualifychecks it (insideshipped_contract_problems). So does the index build (build_registry_index.check_shipped_contract), so a bundle that breaks the rule never ships. The attest job's pinned wheels gain PyYAML for that check.Tests
tests/test_shipped_sql.py: the Odoo SELECT passes, 11 kinds of unsafe query are refused, bindings must be typed inputs, the whole contract is checked, and the index build refuses a bundle whose SQL writes. These pass together with the registry suites (108 passed, 4 skipped withoutsigstore).Also
test_durable_authority_v13tests. Main had already fixed them (b16d6cb). Auto-merge stays on.🤖 Generated with Claude Code