[paynejd]

Linked Issue

Closes OpenConceptLab/ocl_online#553

Summary

openconceptlab 0.2.1 hardens how the client builds request paths and where it sends them. Method signatures don't change, and the ocl command works as before for every identifier OCL accepts.

• Identifiers are encoded as single path segments. Every ID, version, key or queue name the client puts in a path is quoted with quote(value, safe=""). Before, values went into paths as given, so some characters changed which endpoint a request named. An empty value, ., .. or a value containing / raises APIError (status_code 0) before anything is sent. The server would decode a / back into a separator, so no encoding can carry it. A test runs each refused value through 66 calls that cover every path-building method.
• ocl tool recreate-collection builds its own collection paths. It now quotes them, and it refuses an export file whose collection ID would name a different endpoint. Before, a crafted export could make --no-dry-run act on a resource other than the named collection. The collection_url written into the bulk-import lines is unchanged.
• The origin check:
  • compares hosts in their encoded (punycode) form;
  • refuses a URL that carries credentials, which httpx would otherwise send in place of the token;
  • for a base_url with a path prefix, requires the raw path, and every way a server or gateway might read the decoded path, to stay inside the prefix. The readings cover dot segments resolved, backslashes read as slashes, and repeated decoding with ; parameters dropped (capped at four passes). Encoded dot segments, slashes or backslashes can no longer leave the prefix.
• A host that isn't a valid domain name raises APIError. Before, httpx raised idna's error while building the request. Nothing was sent either way.
• search_repos now validates owner_type, like the other methods.
• Exports need a version. An empty one used to make the path name a version called "export".
• fetch_cascade_children sends nothing for an expression with dot segments, ? or #. These expressions come from export files.
• Unchanged: get(endpoint) and the other low-level methods take a whole path as before, and nothing changes in request or response bodies.

Test Plan

• pytest -q: 285 passed (49 before). The new tests failed before the change: requests went out for every refused identifier, and the origin-check cases were allowed. One exception: three tests pin behaviour that already worked (a prefix base_url, a punycode host, an encoded backslash inside the prefix).
• ocl concept get CIEL CIEL "A B+50%" sends /orgs/CIEL/sources/CIEL/concepts/A%20B%2B50%25/ (CLI test). oclapi2 decodes the path once, then matches the raw form and several encoded forms of the identifier, so lookups behave as before.
• ruff check src tests: all checks passed.
• The 0.2.1 wheel installs in a clean environment; import openconceptlab and ocl --version both work.
• CI is green on this PR (Python 3.10 and 3.13).

Notes

• Releasing 0.2.1 to PyPI and tagging v0.2.1 happen after merge.
• The review summaries follow as comments.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QvHhCgtEqAD6xKyJAbToDH

[paynejd]

Codex review, pass 1 (codex-cli 0.163.0, commit 15aa628): 3 findings, all fixed in dc10220.

• One P1: reading backslashes as slashes when checking a base_url path prefix could let a path outside the prefix pass. Now every reading of the path must stay inside it.
• Two P2s:
  • an empty export version dropped out of the path, which then named a different version;
  • a malformed host in a redirect's Location raised idna's error instead of APIError.

Each has a new test. The full pass is on the tracking ticket.

[paynejd]

Codex review, pass 2 (codex-cli 0.163.0, commit dc10220): the pass 1 fixes hold. One new P2, fixed in d03708b: recreate-collection's cascade lookups could follow an export expression whose dot segments named another endpoint on the API. Such expressions are now skipped without a request, and tests cover them. No P0 or P1 is open, so the Codex passes stop. The full pass is on the tracking ticket.

[paynejd]

Simplification review (fresh Claude subagent, commit d03708b): 3 suggestions. I applied two in 12aa8b2: merged two encoding tests, and inlined a one-caller helper. I declined folding the empty-identifier test into the sweep, because the optional versions accept an empty value on purpose. The full review is on the tracking ticket.

[paynejd]

Adversarial review of the token handling (fresh Claude subagent, commit 12aa8b2): one P1 and one P2, both only for a base_url with a path prefix. Both are fixed in 5207d9c.

• The prefix check didn't consider the raw path, so a gateway that routes on the raw path could take a request outside the prefix.
• It also missed paths that are decoded more than once, or carry ; parameters.

The default base_url has no prefix, and with it the reviewer found no way to send the token elsewhere. Seven low-severity notes are recorded on the tracking ticket. The fix is being re-checked.

[paynejd]

Adversarial re-verification (commit 5207d9c): no material issues. The prefix fixes hold in the real code, and legitimate identifiers and prefixed bases still work. Three new low-severity notes, all about a base_url with an unusual path prefix, are recorded on the tracking ticket.

[paynejd]

Codex check of commits dc10220..5207d9c: two P2s, both fixed in 9e2751d:

• a base_url prefix containing ; or a literal escape had stopped working;
• decoding a path until it stopped changing had no bound.

Tests cover both. The details are on the tracking ticket.

[paynejd]

Codex check of 9e2751d: one P2, fixed in b3b7cf8. The prefix comparison missed a path that leaves the prefix only at an intermediate decoding depth. Path and prefix are now compared at every decoding depth, and a test covers it.

[paynejd]

Codex check of b3b7cf8: clean. The review loop is finished: no P0, P1 or P2 is open. The low-severity notes are recorded on the tracking ticket.