🔌 refactor: Decouple bash_tool from Per-User CODE_API_KEY#12712
🔌 refactor: Decouple bash_tool from Per-User CODE_API_KEY#12712danny-avila wants to merge 2 commits intofeat/agent-skillsfrom
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50bb359682
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const codeApiKey = process.env[EnvVar.CODE_API_KEY] ?? ''; | ||
| if (codeApiKey) { |
There was a problem hiding this comment.
Enforce execute_code capability before priming skill files
This path now primes skill files whenever LIBRECHAT_CODE_API_KEY is present in the server environment, but it no longer checks whether code execution is enabled for the run/agent. In deployments where skills are enabled but execute_code is intentionally disabled, invoking a file-backed skill will still upload files to the code sandbox, causing unintended data egress and background code-env activity. Please gate this branch on the same execute-code capability signal used when registering bash_tool.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
| } | ||
| const apiKey = process.env[EnvVar.CODE_API_KEY] ?? ''; |
There was a problem hiding this comment.
Skip history file re-priming when execute_code is disabled
primeInvokedSkills now derives the API key directly from process env and will re-prime historical skill files whenever that env var is set, even if the current run does not have code execution enabled. That means conversations can trigger sandbox uploads during history reconstruction despite bash_tool being unavailable for the agent, which is both unnecessary and a data-scope regression. Add an execute-code capability gate before entering the priming/upload path.
Useful? React with 👍 / 👎.
Addresses review feedback on #12712: the previous commit gated skill file priming only on process.env[EnvVar.CODE_API_KEY] presence, which meant agents with execute_code disabled but the env var present would still upload bundled skill files to the sandbox on every invocation — a data-scope regression. Thread the execute_code capability flag (codeEnvAvailable) through enrichWithSkillConfigurable → configurable, and as a dep on primeInvokedSkills. Both handleSkillToolCall and primeInvokedSkills gate the sandbox-upload path on codeEnvAvailable first; the API key is then read from env inside that gate (if also missing, skip with a warn — admin misconfig). Adds skillFiles.spec.ts pinning the four-way matrix: capability on/off × env key set/unset.
75029e9 to
d519c82
Compare
Phase 4 of Agent Skills umbrella (#12625): gate bash_tool and skill-file priming on the execute_code capability only. LIBRECHAT_CODE_API_KEY is the LibreChat-hosted sandbox service key — system-level, not a per-user secret — so the per-user loadAuthValues lookup was legacy plumbing. The agents library reads process.env[EnvVar.CODE_API_KEY] itself; this change removes the redundant resolution paths.
Addresses review feedback on #12712: the previous commit gated skill file priming only on process.env[EnvVar.CODE_API_KEY] presence, which meant agents with execute_code disabled but the env var present would still upload bundled skill files to the sandbox on every invocation — a data-scope regression. Thread the execute_code capability flag (codeEnvAvailable) through enrichWithSkillConfigurable → configurable, and as a dep on primeInvokedSkills. Both handleSkillToolCall and primeInvokedSkills gate the sandbox-upload path on codeEnvAvailable first; the API key is then read from env inside that gate (if also missing, skip with a warn — admin misconfig). Adds skillFiles.spec.ts pinning the four-way matrix: capability on/off × env key set/unset.
be95291 to
c7c73a2
Compare
Summary
I decoupled
bash_toolregistration and skill-file priming from per-userCODE_API_KEYlookups as part of Phase 4 of the Agent Skills umbrella (#12625).LIBRECHAT_CODE_API_KEYis the LibreChat-hosted sandbox service key — a system-level credential, not a per-user secret — sobash_toolis now gated solely on theexecute_codecapability and the agents library resolves the key fromprocess.envdirectly.enrichWithSkillConfigurableto drop theloadAuthValuesandpreResolvedCodeApiKeyparams and stop injectingcodeApiKeyinto the toolconfigurableobject.primeInvokedSkillsto read the sandbox key fromprocess.env[EnvVar.CODE_API_KEY]directly; removedcodeApiKeyandloadAuthValuesfromPrimeInvokedSkillsDeps.handlers.tsto read the key from env rather thanmergedConfigurable.EnvVarandloadAuthValuesimports fromapi/server/services/Endpoints/agents/initialize.js; dropped the corresponding args at both call sites.bash_toolloader inToolService.jsto callcreateBashExecutionTool({})— the agents library readsLIBRECHAT_CODE_API_KEYfrom the environment internally. Removed the "Code execution is not available" placeholder tool.packages/api/src/agents/skillConfigurable.spec.tswith three focused unit tests pinning the new surface contract.Out of scope (intentionally untouched): the legacy
CodeExecutionToolpath inhandleTools.js, post-execution file-download callbacks, Programmatic Tool Calling registration, and the per-userexecute_codeplugin install hook in the frontend.Change Type
Testing
Ran the targeted Jest suites from
packages/api:npx jest src/agents/__tests__/skills.test.ts— 36/36 pass.npx jest src/agents/__tests__/initialize.test.ts— 9/9 pass.npx jest src/agents/skillConfigurable.spec.ts— 3/3 pass (new).npx jest src/agents— 419 pass; the 6 pre-existing failures (summarization e2e and the handlers session-context passthrough tests) exist on the base branch unchanged, verified viagit stashcomparison. They stem from the parentnode_moduleshaving@librechat/agents@3.1.63installed while the branch expects^3.1.66-dev.0, not from this change.Recommended manual verification in a full install:
LIBRECHAT_CODE_API_KEYset andexecute_codeenabled on an agent with a skill → skill invocation runs bash commands and primes files.execute_codedisabled →bash_toolis absent from the definitions (unchanged behavior).Test Configuration:
feat/agent-skills(HEAD08d02c1dc)Checklist