Skip to content

Odoo new-contact: taught and read back, refused under clinical-write; shipped SQL is one read-only SELECT - #1605

Draft
abrichr wants to merge 4 commits into
feat/skills-registryfrom
registry/odoo-new-contact
Draft

abrichr wants to merge 4 commits into
feat/skills-registryfrom
registry/odoo-new-contact

Conversation

@abrichr

@abrichr abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member

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

  • The fixture. benchmark/odoo_contacts brings 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:
    • wait for Postgres before creating the database;
    • give the dedicated user Contact Creation, without which Odoo shows no save button.
  • The demonstration (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/).
  • Qualification under clinical-write refused 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_problems requires a single parameterized, read-only SELECT. It refuses:

  • a second statement, comments, string literals and ${} text;
  • writing, session, file and sleep functions, including INTO, FOR UPDATE, pg_*, lo_* and dblink;
  • placeholders other than %(name)s, and any binding that isn't {param: ...}.

skill qualify checks it (inside shipped_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 without sigstore).

Also

🤖 Generated with Claude Code

abrichr and others added 3 commits October 28, 2026 16:30
…ll (offline, not yet run)

Pinned, multi-arch images on loopback; a generated admin password, a dedicated user, a read-only record-check role on res_partner, a database-owner oracle and reset by archiving. ERPNext's images are amd64 only, so Odoo comes first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, hooks and benchmark contract

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ipped SQL is one read-only SELECT

The Odoo 18 Community fixture now runs: the demonstration saves a contact
and a parameterized SELECT of the typed fields (keyed by email, through a
read-only role) reads all six back. Qualification under clinical-write
refuses before any case: City and Phone carry one-word labels, and the save
button is an icon with no text (the class #1581 addresses). Evidence and
findings are in benchmark/odoo_contacts.

A shipped SQL read-back must be one parameterized, read-only SELECT
(skills.shipped_sql_problems): no second statement, comments, literals or
${} text; no writing, session, file or sleep functions; every %(name)s bound
to a typed input. skill qualify and the index build both check it; the
attest job's pinned wheels gain PyYAML for that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

Adversarial review at 44c8ee5 (stacked on #1592)

Verdict: no blockers. This PR adds no registry entry, so nothing it ships can end in a verified write. The Odoo fixture is synthetic, and the refusal under clinical-write is the correct outcome.

Tests. test_shipped_sql.py and the #1592 registry suites pass: 115 passed. The one failure is the same mcp 2.x issue in the borrowed venv that I noted on #1592.

Not blocking: shipped_sql_problems checks words, not effects

This check will run when a pulled skill ships SQL, so it should hold up before the first SQL-carrying entry ships. Each of the queries below passes at 44c8ee5:

P = {"email": {"param": "email"}, "name": {"param": "name"}}
# writes a sequence: "setval" and "nextval" don't match \bset\b
"SELECT setval(%(name)s::regclass, 1) AS id, nextval(%(email)s::regclass) AS email"
# changes a session setting
"SELECT set_config(%(name)s, %(email)s, false) AS id"
# runs SQL taken from a typed input
"SELECT query_to_xml(%(name)s, true, true, %(email)s)::text AS id"
# echoes the inputs as the "stored" fields: any write matches every field
"SELECT id::text AS id, %(name)s AS name, %(email)s AS email FROM res_partner "
"WHERE id = (SELECT max(id) FROM res_partner)"

The last query matters most under the threat model. It still reads a new row, so the count check (RECORD_WRITTEN) passes, but every field check then compares the inputs with themselves. A wrong save would be reported as verified. Today this would need a bad contract from the publisher, which is why it isn't blocking.

Fix:

  1. Open the shipped check's connection read-only at the driver (conn.set_session(readonly=True) for psycopg2, or default_transaction_read_only=on in the options), instead of trusting the word list.
  2. Allow placeholders only after WHERE. Require the select list to be plain column references, optionally with ::text or AS.
  3. Refuse function calls in the select list, except for an allowlisted set of casts.

Add these four queries to test_shipped_sql.py as refusals.

Other points

  • The match key. Duplicate emails are normal in Odoo, and the Odoo SELECT is keyed by email ... AND active. The before-and-after count saves it on a real instance. Still, add create_date >= %(started_at)s, or let the user's binding narrow the query, before this becomes a registry entry.
  • README. It says results/2026-10-28-click-teach/ holds "the compiled bundle and the qualification evidence". Only qualification/ is committed.
  • Internal details. qualify.log ends with a local path (~/oa/scratch/second-installation/...). The README's Podman disk-resize notes are internal process. Drop both from the public tree.

Open core

The secrets are generated in state_dir, outside the repo. The images are pinned by digest and nothing of Odoo is vendored (Odoo is LGPL-3.0). The data is synthetic (example.invalid). There are no tuned thresholds, corpora or real datasets. No prospect names appear.

Mergeability

Mergeable once #1592 lands and this PR is retargeted to main. It only adds files and checks.

The read-back now runs on a driver-level read-only session (PRAGMA
query_only, psycopg read-only, a READ ONLY MySQL session), and a shipped
contract may name only drivers that support it. A shipped query 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.

The Odoo benchmark drops a local scratch path from qualify.log and the
fixture default, drops internal VM notes from the README, and says the
compiled bundle isn't committed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant