Embeddable auth and saved queries, plus stricter URL key validation (0.7.0) - #27
Merged
Conversation
…=...) Every existing provider *acquires* a token. Some embedders already hold one and only need the SDK to use it: a hosted service doing an on-behalf-of exchange per request so BC calls run as the signed-in user, a CI job handed a token by its platform, a notebook pasting one in. None of those can use the browser flow, which opens a browser and binds a loopback listener on whichever machine runs the process. StaticTokenAuth takes a fixed token or a supplier. Prefer the supplier in a long-running process: it is re-invoked on every call, so a refreshed token is picked up rather than one pinned at construction expiring mid-session. Nothing is cached, so clear_cache() is a no-op and the caller owns the token lifetime. It validates at the boundary — an empty token and a "Bearer " prefix both raise ConfigError rather than surfacing as a confusing 401 several layers away. AsyncBCClient(auth=...) short-circuits _build_auth, so the profile's auth_method is never consulted. Everything else still comes from the profile: environment, company, registry, and the disable_standard_api gate. Credential acquisition moves out; routing and its security properties do not.
test_no_context_policy_path asserted against the developer's actual ~/.config/bcli, so it failed on any machine that had ever recorded a bcli error and passed only on a clean CI home. It mimics `bcli ask --no-context` but omitted the skip_last_error=True that flag actually passes (ask_cmd.py), and unlike every sibling test in the file it did not isolate config_dir. Do both.
bcli already runs on 3.14 (the dev and tool environments are 3.14.3) but the classifier list stopped at 3.13, so the metadata understated what is tested. requires-python stays >=3.11 deliberately. This is a published package and most production Python is still 3.11-3.13; raising the floor would exclude those users for no benefit, since a lower floor installs cleanly on 3.14 anyway.
Saved-query handling lived entirely in bcli_cli/commands/query_cmd.py, so
nothing outside the CLI could run a saved query without importing Typer. An
embedder — a service, a notebook, another tool — had to reimplement catalog
loading, parameter validation and placeholder resolution, which is exactly the
code you least want duplicated: validating params against their declared
type/pattern/min/max/enum *before* any HTTP is what keeps caller input out of
OData filter syntax.
bcli.queries now owns that, split by concern: _catalog (load and index a YAML
catalog), _params (validate supplied params against the declared schema),
_expand (resolve ${{ params.X }} and escape values), _errors (a small typed
hierarchy). query_cmd.py becomes a consumer and loses 291 lines; observable CLI
behaviour, output and exit codes are unchanged.
QueryCatalogError, QueryError and QueryParamError are exported from the package
root so an embedder can distinguish a bad catalog from a bad parameter.
61 new tests.
…mponents
build_url validated the custom-API route segments but spliced entity_set_name
and record_id in raw. Both land directly in the URL path, and a record key is
caller-influenced anywhere one is accepted from outside, so a raw '/' in it
starts a new path segment:
entity_set_name="engineOverviews"
record_id="1)/../../../../../../glEntries('X'"
→ .../engineOverviews(1)/../../../../../../glEntries('X')
which collapses to glEntries('X'). Every earlier check saw only
"engineOverviews" — including the endpoint-registry lookup and the
disable_standard_api gate, both of which key on the entity-set name alone. So a
profile restricted to a curated registry could still address an entity outside
it. Business Central applies the caller's own permission set regardless, so this
does not reach data the user could not otherwise read, but the client-side
restriction was not doing what it claimed.
validate_record_key rejects raw '/', '\', '?' and '#' plus the '.'/'..'
segments, and points the caller at percent-encoding. Quotes, commas, equals
signs, hyphens and parentheses inside quoted strings ('ACME (US)', 'O''Brien',
composite k1='a',k2='b') all still pass — none of them can start a path
segment.
Applied in build_url and in _parse_bound_action, the latter because the
bound-action resolver splices (key)/Namespace.Action onto a parent URL it
resolved from the registry while consulting the registry about the parent only.
Its comment asserted the tail was "opaque to the registry ... gated on the
parent, which is the security-relevant identity"; that is true only once keys
cannot carry path syntax, which it now is.
26 new tests, including the reported payload and the legitimate key shapes.
CI runs 'uv sync --locked', which refuses to re-resolve. bcli is a member of its own lockfile, so bumping the project version left uv.lock stale and every matrix job failed in the install step before running a single test.
This was referenced Aug 4, 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
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.
Overview
Three things, all needed to let something other than the CLI drive the SDK, plus a
correctness fix found while reviewing that work.
The motivating consumer is a hosted service that authenticates a user itself and then
calls BC on their behalf. It couldn't use the SDK at all: every auth provider
acquires a token, and the interactive one needs a browser and a loopback listener on
whichever machine runs the process. And it couldn't run saved queries without
importing the Typer CLI.
Changes
StaticTokenAuth+AsyncBCClient(auth=...)— supply a token you already hold.Accepts a string or a callable; a callable is re-invoked per request so a long-running
process picks up a refreshed token rather than pinning one that expires. Validates at
the boundary: an empty token and a
Bearerprefix both raise rather than surfacingas a confusing 401 three layers away. Injected auth short-circuits
_build_auth, sothe profile's
auth_methodis never consulted — everything else still comes from theprofile, including the registry and the
disable_standard_apigate.bcli.queries— catalog loading, parameter validation against each parameter'sdeclared
type/pattern/min/max/enum, and${{ params.X }}resolution withOData escaping, split across
_catalog/_params/_expand/_errors.query_cmd.pybecomes a consumer and drops 291 lines. Validating parameters beforeany HTTP is the property worth not duplicating, which is why this moved rather than
being reimplemented downstream. CLI output, errors and exit codes are unchanged.
build_urlvalidatesentity_set_nameandrecord_idas single path components.Both are spliced straight into the request path; a record key containing
/starts anew path segment, so
composed a URL addressing
glEntries('X')while the registry lookup and thedisable_standard_apigate had only ever seenengineOverviews— both key on theentity-set name alone. BC applies the caller's own permission set regardless, so this
did not reach data a user couldn't otherwise read, but the client-side restriction
wasn't doing what it claimed. Now rejects raw
/,\,?,#and./.., andpoints at percent-encoding. Real keys are unaffected — GUIDs, integers,
'V00010','O''Brien','ACME (US)',k1='a',k2='b'all still pass, since none of them canstart a path segment.
The same validation applies to the key inside a bound-action invocation. That
resolver splices
(key)/Namespace.Actiononto a parent URL and consults the registryabout the parent only; its comment claimed the tail was "opaque to the registry ...
gated on the parent, which is the security-relevant identity", which holds only once
keys can't carry path syntax.
Also: Python 3.14 added to the classifiers (
requires-pythonstays>=3.11— a lowerfloor installs fine on 3.14 and raising it would drop 3.11–3.13 users), and a test
that asserted against the developer's real
~/.config/bclinow isolates its configdir, so it passes on a machine that has recorded a bcli error instead of only on a
clean CI home.
Test plan
uv run pytest— 1060 passed, 5 skipped (was 1034; +26 URL validation, +61 queries, and the pre-existing suite unchanged)uv run ruff check src tests— cleanStaticTokenAuthverified end-to-end from a separate project's venv: abrowserprofile builds a transport with the injected provider, no browser opened, no loopback listener boundquery_cmd.pyrefactor