fix: Port skill resource path and skill name validation fixes to v1 - #6805
Merged
Conversation
The wrapper script generated by _SkillScriptCodeExecutor joined each skill resource path onto the extraction directory as-is, so a resource named "../../pwned" resolved outside that directory and its content was written there before the skill script ran. The generated code now normalizes each relative path, raises PermissionError when the result starts with ".." or is absolute, and joins the remainder onto the absolute extraction directory. Nested paths such as "subdir/notes.md" still materialize. A resource whose filename itself begins with two dots, for example "..config", is refused as well; this matches the upstream fix. Ports the extraction hunk of upstream PR #5927. The unrelated argument-validation refactors in that commit are not included.
…e path (v1)
GCPSkillRegistry.get_skill() interpolated the caller-supplied name straight
into "projects/.../locations/.../skills/{name}" and handed that to the Vertex
SDK, so a name containing a slash or "../" addressed a different resource than
the one asked for. The name comes from a model-issued tool call, so nothing
upstream of this point constrains it.
It now checks the name against the same snake-or-kebab pattern skill names are
already held to, and raises ValueError before any request is made.
Behaviour change: a name outside that character set, such as one with an
uppercase letter, a dot, or a slash, now raises ValueError locally instead of
reaching the registry. The registry would have rejected it anyway, so what
changes is the error type and the fact that no request goes out.
Ports the validation half of the upstream change. The percent-encoding half
does not apply here, because this branch builds an SDK resource name rather
than a URL and every character the pattern accepts is already safe.
GWeale
changed the title
wukath
approved these changes
Aug 19, 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
2 participants
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.
This PR ports two skill-handling fixes to the
v1branch:Skill resource path validation (
2f15c6cb)directory; anything else fails the script run with
PermissionError..., such as..config, is refused too;rename the file.
subdir/notes.mdstill materialize.Skill name validation (
11101acc)GCPSkillRegistry.get_skill()requires a name matchingmodels._SNAKE_OR_KEBAB_NAME_PATTERN, and raisesValueErrorbefore theresource path is built and before the client is called.
a-z,0-9,-and_.Neither change has an opt-out.