Repository navigation
feat(task_step): pass per-run secrets by reference, and keep raw ones out of the run index - #235
Open
AlexS-scale wants to merge 2 commits into
Open
AlexS-scale wants to merge 2 commits into
AlexS-scale wants to merge 2 commits into
Conversation
… out of the run index A per-run key in user_overrides is stripped from every persisted context, so a step that runs on another host than the one the run was submitted from never sees it. A bare secret:KEY or env:NAME reference names a secret rather than holding it, so redaction now keeps it, and the steps that read a per-run key resolve it with Config.resolve_reference(). A reference with a ?default may carry the secret, so it is still stripped. The run index stored a run's overrides verbatim, so a key sent in the explorer's overrides or metadata reached the runs collection and its list endpoint. insert_run strips the redacted keys; LocalRunner seeds the run from its in-memory record, so the run still gets the key.
…s for it An install whose required_params do not name litellm_api_key no longer depends on the per-run key's reference resolving on this host.
This branch has not been deployed
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
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.
What and why
Two problems with per-run secrets (
litellm_api_key,judge_litellm_api_keyincontext.metadata["user_overrides"]).A step on another host never sees them. Redaction strips them from every persisted context, which is right. But a
[runner]whose steps run in a different process from the submitter can only rebuild the context from storage, so the key is gone.LocalRunneris unaffected only because it holds the live context in memory.The run index stored them in plaintext.
insert_runwroteRunRecord.overridesverbatim. A key sent through the explorer'soverrides(or smuggled throughmetadata.user_overrides) landed in therunscollection and came back fromGET /tasks/{id}/runs.Changes:
_is_redacted(key, value): a baresecret:KEYorenv:NAMEreference names a secret rather than holding it, soto_safe_dict()and the context-ops diff keep it. A reference with a?defaultmay carry the secret itself, so it is still stripped.Config.resolve_reference(value)resolves anenv:/secret:reference (the latter through[stores.secret]) and passes anything else through. The five places that read a per-run key use it:deploy_agentdeploy_envinstall_agentrubrics_verifierinsert_runstrips_REDACTED_KEYSfrom the overrides it persists.LocalRunnerseeds the run from its in-memory record, so the run still gets the key.--litellm-api-keyand--judge-litellm-api-keyontask run/task run-batchsay they take a reference, which also keeps a key out of shell history.Trust boundary. A reference resolves against the environment and secret store of the host running the step. In this repository every run submitter (CLI user, loopback explorer user) already has that access, so nothing is newly exposed. A runner that takes runs from callers who may not read those must refuse references it did not create; AGENTS.md says so.
How it was tested
pytest tst/unit -n auto: 7899 passed. The 2test_mcp_cli_builder.pyfailures are environmental: they pass with the venv onPATH(13 passed).install_agentinstalls with the value asecret:reference names in the secret store.to_safe_dict()and the journal; one with a?defaultdoes not.Config.resolve_referenceresolvesenv:and passes other values andNonethrough.ruff checkpasses.check_plugin_api.py --base origin/main: no break.

Confidence Score: 5/5
The PR appears safe to merge; the previous install failure is fixed.Summary
The PR keeps bare secret references in saved contexts and removes raw per-run keys from the run index.
Reviews (2) · Last reviewed commit: "fix(install_agent): resolve the per-run ..." · Reviewed by Greptile