You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit ba0c406
Browse filesBrowse the repository at this point in the historyBrowse files
fix(obs): correct litellm routing, and close the public-PyPI fallback in templates
Four review findings from @harvhan on #523. Both blocking ones confirmed by
measurement first; the two non-blocking ones handled differently, see below.
**groq and deepseek were being dropped (blocking).** They appear in litellm's
`openai_compatible_providers`, so the previous commit marked them as reaching
the model over the openai client and stood down — but litellm catches both in an
EARLIER branch of its dispatch chain that calls `base_llm_http_handler`, its own
HTTP client, which the OpenAI instrumentor never sees. Nothing recorded them.
That list describes API-FORMAT compatibility, not which Python client is used;
membership is necessary but not sufficient. Eight of the 54 are intercepted this
way: azure_ai, cometapi, deepseek, fireworks_ai, groq, hosted_vllm, ragflow,
xai. They are now subtracted, and a drift test re-derives the set from the
installed litellm's dispatch chain so an upgrade that moves a provider between
branches fails CI instead of silently losing or doubling its metrics. Verified
the guard bites: removing groq from the set fails with the provider named.
**Scaffold builds could fall back to public PyPI (blocking).** `Dockerfile-uv.j2`
exported `UV_INDEX="scale-pypi=<broker url>"`, which re-binds a NAMED index but
does not make it the default — and the project's own `default = true` does not
survive the re-bind, so PyPI stayed in the list. A package missing from the
mirror was then fetched publicly. Measured against a local index that 404s
every package, resolving `idna`:
pyproject default = true, no override no solution correct
UV_INDEX="scale-pypi=<broker url>" RESOLVED FROM PYPI the hole
UV_DEFAULT_INDEX=<broker url> no solution correct
All 19 uv templates now export `UV_DEFAULT_INDEX`, matching what `Dockerfile.j2`
already did. This also keeps the property the named index was chosen for in the
first place — not letting a project's own pyproject redirect where the token is
sent. Verified with two local servers, the project pointing `scale-pypi` at a
rogue host: rogue received zero requests, only the broker URL was contacted. The
token stays percent-encoded, which is correct inside URL userinfo, so the decode
step and the name-bound credential vars are gone. PRIVATE_INDEX.md rewritten —
its "the two variants work differently, deliberately" section is no longer true.
**GenAI tracing could not be switched off on its own (non-blocking).** The
openai-agents bridge is installed by this package, not by `sgp_obs.init`, so
there was no way to stop model spans short of disabling traces entirely. Gated
on SGP_OBS_GENAI_ENABLED, defaulting ON so an unset variable changes nothing.
Caveat: that variable name could not be verified against sgp-obs from this repo
(it is not a dependency and not on public PyPI), and it appears nowhere in this
codebase. If the real name differs, this gate simply never fires and behaviour
is unchanged — but it is worth confirming.
**The shutdown budget does not cover provider atexit hooks (non-blocking).**
Confirmed: TracerProvider and MeterProvider both default `shutdown_on_exit=True`
and do `atexit.register(self.shutdown)`, so if the budget expires those hooks
still flush at interpreter exit. NOT fixed here, deliberately — unregistering
them would also strip runtime-owned providers that sgp-obs leaves alone on
purpose. Documented as a known limit with the fix that belongs in sgp-obs:
build its own providers with `shutdown_on_exit=False`, or give `shutdown()` a
deadline.
Tests: 97 across the two obs suites, 1714 repo-wide. `./scripts/lint` (ruff,
pyright 0 errors, import) and `./scripts/check-slim-deps` clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The name must be exactly `scale-pypi`. uv applies `UV_INDEX_SCALE_PYPI_USERNAME` /
29
-
`UV_INDEX_SCALE_PYPI_PASSWORD` to the index of that name, so renaming it makes the credentials
30
-
silently stop applying. Setting `UV_INDEX_URL` instead does not authenticate a *named* index at
31
-
all, and the resolve fails with a 401.
28
+
`default = true` is the part that matters: it replaces PyPI rather than adding to it, so a
29
+
package missing from the mirror fails the build instead of being fetched from PyPI. The
30
+
Dockerfiles preserve that property (see below). The name is free-form now; nothing binds
31
+
credentials to it.
32
32
33
33
## Three things that are easy to get wrong
34
34
35
-
**The token arrives percent-encoded.**The buildspec URL-encodes it to embed it in the pip config's
36
-
URL userinfo, so a token containing `+`, `/` or `=` arrives as `%2B`, `%2F`, `%3D`. The `uv sync`
37
-
templates decode it before exporting it as a password. Passing it through still-encoded sends a
38
-
different string and the resolve 401s.
35
+
**Use `UV_DEFAULT_INDEX`, never `UV_INDEX`.**`UV_INDEX` adds or re-binds a *named* index but
36
+
does not make it the default, so public PyPI stays in the list as a fallback — and the project's
37
+
own `default = true` does not survive the re-bind. Measured against a local index that 404s every
38
+
package, resolving `idna`:
39
39
40
-
**The credential must not follow project-controlled configuration.** uv binds credentials by index
41
-
*name*, and the name-to-URL mapping would otherwise come from the agent's own `pyproject.toml` — so a
42
-
project that pointed `scale-pypi` at another host would receive the token. Verified against a local
43
-
server: the rogue host receives `Authorization: Basic aws:<token>` and the real index is never
44
-
contacted. The templates therefore export `UV_INDEX` to re-bind the name to the URL the *broker*
45
-
supplied, which overrides whatever the project declared. With that in place the rogue host is never
46
-
contacted. The pinned URL carries no userinfo; the token still travels only in
47
-
`UV_INDEX_SCALE_PYPI_PASSWORD`.
40
+
| Configuration | Result |
41
+
| --- | --- |
42
+
|`pyproject``default = true`, no override | no solution — correct |
43
+
|`UV_INDEX="scale-pypi=<broker url>"`|**resolved from public PyPI**|
44
+
|`UV_DEFAULT_INDEX=<broker url>`| no solution — correct |
48
45
49
-
The case this defends is not a malicious agent author — they also write the Dockerfile and could read
50
-
the mounted secret directly. It is a *contributed* change to a project file, where a one-line URL edit
51
-
is far less conspicuous in review than an exfiltration command in a Dockerfile.
46
+
So the middle row silently widened the supply chain to all of PyPI for any package the mirror
47
+
does not carry. Both install steps in `Dockerfile-uv.j2` now export `UV_DEFAULT_INDEX`.
52
48
53
-
**The two template variants work differently, deliberately.**
49
+
**The credential must not follow project-controlled configuration.** The index URL has to come
50
+
from the injected secret, not from the agent's `pyproject.toml` — otherwise a project that
51
+
pointed its index at another host would receive the CodeArtifact token. `UV_DEFAULT_INDEX`
52
+
satisfies this by construction, because the URL it carries comes wholly from the broker.
53
+
Verified with two local servers, the project declaring `scale-pypi` at a rogue host and
54
+
`UV_DEFAULT_INDEX` set to the broker's: the rogue host received **zero** requests and only the
55
+
broker URL was contacted.
54
56
55
-
| Template | Install step | How the credential is supplied |
56
-
| --- | --- | --- |
57
-
|`Dockerfile-uv.j2`|`uv sync` against the agent's `pyproject.toml`| Named index `scale-pypi`, pinned via `UV_INDEX`, token decoded into `UV_INDEX_SCALE_PYPI_PASSWORD`|
58
-
|`Dockerfile.j2`|`uv pip install -r requirements.txt`| No pyproject is present, so there is no named index to bind to. The credentialed URL is used directly via `UV_DEFAULT_INDEX`|
57
+
The case this defends is not a malicious agent author — they also write the Dockerfile and could
58
+
read the mounted secret directly. It is a *contributed* change to a project file, where a
59
+
one-line URL edit is far less conspicuous in review than an exfiltration command in a Dockerfile.
60
+
61
+
**The token stays percent-encoded.** The buildspec URL-encodes it to embed it in the pip config's
62
+
`index-url` userinfo, so a token containing `+`, `/` or `=` arrives as `%2B`, `%2F`, `%3D`. Both
63
+
templates now pass that URL through untouched, which is correct: it is already encoded for
64
+
exactly this use, and decoding it would corrupt it. (An earlier revision of `Dockerfile-uv.j2`
65
+
decoded it into `UV_INDEX_SCALE_PYPI_PASSWORD`, which was necessary only because credentials bind
66
+
to a *named* index. That mechanism is gone.)
59
67
60
-
The `requirements.txt` variant does **not** decode the token, and that is the point: it stays inside
61
-
the URL, already encoded for exactly that use. Decoding it there would corrupt it. It is also not
62
-
exposed to the redirection problem above, because the URL comes wholly from the injected secret.
68
+
**Both template variants now work the same way.**
69
+
70
+
| Template | Install step | How the index is supplied |
71
+
| --- | --- | --- |
72
+
|`Dockerfile-uv.j2`|`uv sync` against the agent's `pyproject.toml`|`UV_DEFAULT_INDEX`, credentialed URL straight from the secret |
73
+
|`Dockerfile.j2`|`uv pip install -r requirements.txt`|`UV_DEFAULT_INDEX`, credentialed URL straight from the secret |
0 commit comments