Skip to content

Let Fishbowl use its explicit certificate-verification-off setting - #5827

Merged
shiny-code-app[bot] merged 5 commits into
mainfrom
work/lp5810-fishbowl-verify-off
Nov 9, 2026
Merged

shiny-code-app[bot] merged 5 commits into
mainfrom
work/lp5810-fishbowl-verify-off

Conversation

@shiny-code-app

@shiny-code-app shiny-code-app Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

The Director decided in Q1327 that the trusted, private Fishbowl source connects without verifying the server certificate. Its certificate names no host, so strict verification could never pass. The managed fishbowl.ssl_verify setting already says this, but three of our own checks refused it.

Refs #5810. The one real grants read happens after this deploys, so this PR does not close the issue.

What changed

  • The fixed SHOW GRANTS probe skips certificate checks only when it is sent the explicit off signal. The default stays verified.
  • The host-bound Fishbowl read accepts false or 0 and passes that to the probe on stdin, next to the credentials that already travel there. An unrecognized value still stops at settings_tls.
  • The import consumer path accepts verification off for Fishbowl only, and only from an explicit false or 0. Preview delivery and the runtime grant read both use it. This matches what the Fishbowl importer already does with that setting.
  • The service-origin read stays strict, and every other source stays verified.

No account, password, grant, trust store, pin or plaintext mode is added. TLS still applies, because the server offers it.

Removed: the strict refusal of fishbowl.ssl_verify=false in the host probe and consumer path, and the stale "TLS verification is mandatory" comment.
Net lines: +123 (source and docs +25, tests +95, Dockerfile +3, CI pin 0). Growth is the two-condition exception this decision needs and the behavior tests that pin it to Fishbowl.

Verification

Classification: credential and TLS safety. Whole modules run locally: tests.test_odoo_host_source_grants, tests.test_odoo_import_diagnostics, tests.test_odoo_runtime_source_grants, tests.test_odoo_runtime_source_prerequisites, tests.test_odoo_preview_import_source, tests.test_production_consumer_reads, tests.test_product_secret_copy. All pass. The new tests failed before the fix. Ruff, mypy, the catalog TLS annotation check and the comment check pass.

Inspection: native PyCharm returned UNKNOWN with native_python_support_plugin_unavailable (cbusillo/codex-skills#3363), with no findings reported.

Outside review (OpenAI, one round): one P2 finding, accepted. A misspelled setting would have counted as verification off in the consumer path, so that path now requires an explicit false or 0, with a test.

The comment gate in CI is pinned to a catalog commit older than its # insecure-tls: support, so this PR moves that one pin to the catalog commit that accepts the annotation the TLS policy requires.

The image scan began failing on a new golang.org/x/net advisory (CVE-2026-78661) in the two Go binaries the image builds, which blocks every branch. The Dockerfile moves that module to the fixed v0.62.0, plus the x/crypto and x/text versions that release requires.

Plain TLS fallback is not added. PyMySQL requires TLS once it is given a context, the same way the importer connects, and this server offers TLS.

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 6166f93

Outside review, one round: OpenAI, model not reported by the CLI.

Acted on, reproduced by a test that failed first: a misspelled fishbowl.ssl_verify was treated as verification off in the consumer path, because the installed importer parses any unrecognized text as false. The consumer path now requires an explicit false or 0, the same as the host read. Fixed in 6cad5bd.

Declined: none. The commit after that fix removes a redundant test cast and changes no behavior.

Inspection on the current change: UNKNOWN, native_python_support_plugin_unavailable (cbusillo/codex-skills#3363), with no findings reported.

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 84d6d90

Outside review, one round: OpenAI, model not reported by the CLI.

Acted on, reproduced by a test that failed first: a misspelled fishbowl.ssl_verify was treated as verification off in the consumer path, because the installed importer parses any unrecognized text as false. The consumer path now requires an explicit false or 0, the same as the host read. Fixed in 6cad5bd.

Declined: none. The commit after that fix removes a redundant test cast and changes no behavior.

Inspection on the current change: UNKNOWN, native_python_support_plugin_unavailable (cbusillo/codex-skills#3363), with no findings reported.

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 8d9c2ac

Outside review, one round: OpenAI, model not reported by the CLI.

Acted on, reproduced by a test that failed first: a misspelled fishbowl.ssl_verify was treated as verification off in the consumer path, because the installed importer parses any unrecognized text as false. The consumer path now requires an explicit false or 0, the same as the host read. Fixed in 6cad5bd.

Declined: none. The commit after that fix removes a redundant test cast and changes no behavior.

Inspection on the current change: UNKNOWN, native_python_support_plugin_unavailable (cbusillo/codex-skills#3363), with no findings reported.

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Ready for the merge train

Head: 8d9c2ac

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Launchplane completed the merge-train step for this pull request.

Launchplane finished the merge-train step for this pull request.

Launchplane manages this comment and will update it as the train moves.

shiny-code-app Bot added a commit that referenced this pull request Nov 9, 2026
@shiny-code-app
shiny-code-app Bot merged commit 363d3f9 into main Nov 9, 2026
49 checks passed
@shiny-code-app
shiny-code-app Bot deleted the work/lp5810-fishbowl-verify-off branch November 9, 2026 15:25
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.

0 participants