Skip to content

Escape dynamic values in the customizations index - #327475

Merged
Paul (pwang347) merged 2 commits into
microsoft:mainfrom
sricursion:codex/fix-customizations-index-escaping
Jul 30, 2026
Merged

Escape dynamic values in the customizations index#327475
Paul (pwang347) merged 2 commits into
microsoft:mainfrom
sricursion:codex/fix-customizations-index-escaping

Conversation

@sricursion

@sricursion Sriraj (sricursion) commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #327467

What changed

  • XML-escape the customizations-index text fields requested in review: name, description, applyTo, and argumentHint.
  • Apply the same producer-side fix in both the core and built-in Copilot implementations, including generated nested-AGENTS.md descriptions and the truncated skill-name fallback.
  • Add regressions showing that adversarial closing-tag payloads stay encoded text and cannot create additional instruction, skill, or agent records.
  • Keep file-path serialization and all consumers unchanged; this patch adds no format marker, decoding, parser, authorization, or cache behavior.

Validation

  • npm run typecheck-client
  • npm run transpile-client
  • Core computeAutomaticInstructions.test.ts: 64 passing
  • npm run eslint
  • npm run valid-layers-check
  • npm run define-class-fields-check
  • Copilot npm run typecheck
  • Copilot npm run lint
  • Copilot npm run compile
  • Copilot focused automaticInstructionsCollector.spec.ts: 32 passing
  • Copilot full npm run test:unit: 9,553 passing, 129 skipped
  • git diff --check

Copilot AI review requested due to automatic review settings July 25, 2026 17:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sricursion
Sriraj (sricursion) marked this pull request as ready for review July 25, 2026 17:18
@pwang347

Copy link
Copy Markdown
Member

Hi Sriraj (@sricursion), thank you for opening a PR!

Could we please scope this down to just escaping the input text fields (ie., wrapping name, description, applyTo, argumentHint)? We should try to keep the changes here simple. Thanks!

@sricursion

Copy link
Copy Markdown
Contributor Author

Thanks for the guidance Paul (@pwang347) . I’ve scoped the PR down accordingly. It now only escapes name, description, applyTo, and argumentHint in both customizations-index producers.

I removed the format-marker, decoder, parser, cache, authorization, and file-path changes. Focused regression tests cover structural-tag injection through the retained text fields, and the typechecks, compilation, lint, focused tests, and full Copilot unit suite pass. The PR description has also been updated to reflect the narrower change.

@pwang347 Paul (pwang347) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thank you for contributing!

@pwang347
Paul (pwang347) enabled auto-merge (squash) July 29, 2026 23:46
@pwang347
Paul (pwang347) merged commit e530e14 into microsoft:main Jul 30, 2026
44 of 47 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.132.0 milestone Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workspace skill descriptions can alter customization-index structure and suppress external-file confirmation

5 participants