Repository navigation
fix: require external telemetry allowlists - #3514
Merged
Tom Meschter (tmeschter) merged 1 commit intoNov 11, 2026
Merged
Tom Meschter (tmeschter) merged 1 commit into
Tom Meschter (tmeschter) merged 1 commit into
Conversation
Remove embedded allowlist resources and fallbacks, preserve file validation, and update native smoke callers and maintenance tooling. Fixes microsoft#3455 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the change to externally supplied telemetry allowlists.
No open issues 🎉
What changed in this PR
This PR addresses #3455 by making the standalone telemetry reporter rely exclusively on external allowlists supplied by plugin packages, so allowlist updates no longer require rebuilding the reporter.
Changes:
- Require all three allowlist paths and remove embedded snapshots and fallback loading while preserving validation and normalization behavior.
- Update tests and native smoke callers to use explicit allowlist files.
- Preserve shared tool-list history, retire snapshot synchronization, and document the external-file contract.
| File | Description |
|---|---|
telemetry-reporter/tests/Ghcfa.Telemetry.Tests/Validation/PluginTelemetryValidatorTests.cs |
Uses file fixtures for validation tests. |
telemetry-reporter/tests/Ghcfa.Telemetry.Tests/Validation/PluginTelemetryAllowlistTests.cs |
Tests file-only loading and absence of embedded lists. |
telemetry-reporter/tests/Ghcfa.Telemetry.Tests/TemporaryAllowlists.cs |
Adds reusable temporary allowlist fixtures. |
telemetry-reporter/tests/Ghcfa.Telemetry.Tests/Telemetry/PluginTelemetryActivityWriterTests.cs |
Supplies required paths in test options. |
telemetry-reporter/tests/Ghcfa.Telemetry.Tests/Telemetry/OpenTelemetryPluginTelemetryPublisherTests.cs |
Supplies required paths in publisher tests. |
telemetry-reporter/tests/Ghcfa.Telemetry.Tests/PluginTelemetryApplicationTests.cs |
Covers missing paths, file rereading, and empty lists. |
telemetry-reporter/tests/Ghcfa.Telemetry.Tests/CommandLine/PluginTelemetryCommandLineParserTests.cs |
Verifies all allowlist options are required. |
telemetry-reporter/src/Ghcfa.Telemetry/Validation/PluginTelemetryValidator.cs |
Loads external allowlists for each validation. |
telemetry-reporter/src/Ghcfa.Telemetry/Validation/PluginTelemetryAllowlist.cs |
Removes embedded loading and fallback behavior. |
telemetry-reporter/src/Ghcfa.Telemetry/PluginTelemetryApplication.cs |
Constructs the file-based validator. |
telemetry-reporter/src/Ghcfa.Telemetry/Models/PluginTelemetryOptions.cs |
Makes allowlist paths required properties. |
telemetry-reporter/src/Ghcfa.Telemetry/Ghcfa.Telemetry.csproj |
Removes embedded resource inclusion. |
telemetry-reporter/src/Ghcfa.Telemetry/CommandLine/PluginTelemetryCommandLineParser.cs |
Requires and binds all three paths. |
telemetry-reporter/src/Ghcfa.Telemetry/CommandLine/HelpText.cs |
Documents required external allowlists. |
telemetry-reporter/resources/allowed-tool-names.json |
Deletes the embedded tool snapshot. |
telemetry-reporter/resources/allowed-skill-names.json |
Deletes the embedded skill snapshot. |
telemetry-reporter/resources/allowed-plugin-file-references.json |
Deletes the embedded reference snapshot. |
telemetry-reporter/README.md |
Explains external-file usage and maintenance. |
telemetry-reporter/eng/scripts/Build-Native.ps1 |
Creates, mounts, and cleans smoke-test allowlists. |
telemetry-reporter/AGENTS.md |
Updates reporter development guidance. |
scripts/src/generate-azure-mcp-allowlist.ts |
Requires the existing shared list to preserve history. |
scripts/src/__tests__/telemetry-packaging.test.ts |
Tests smoke fixture paths, cleanup, and telemetry settings. |
scripts/src/__tests__/sync-allowlists-workflow.test.ts |
Removes tests for the retired workflow. |
scripts/src/__tests__/generate-azure-mcp-allowlist.test.ts |
Tests history preservation and missing-source failure. |
docs/hooks.md |
Documents required paths and failure behavior. |
docs/azure-mcp-allowlists.md |
Updates shared-list maintenance and hook documentation. |
.github/workflows/sync-telemetry-reporter-allowlists.yml |
Removes obsolete snapshot synchronization. |
.github/copilot-instructions.md |
Updates repository guidance for external allowlists. |
Review effort: Balanced
JasonYeMSFT (JasonYeMSFT)
approved these changes
Nov 11, 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
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.
Description
The telemetry reporter still bundles skill, reference, and tool allowlists and permits omitted file paths to fall back to those snapshots. This duplicates the packaged plugin allowlists and requires rebuilding the reporter to update them.
Require
--allowed-skills,--allowed-references, and--allowed-tools, and load all three files for each invocation before validating or publishing events. Remove the embedded resources and fallback paths while preserving empty-list semantics, strict grouped tools JSON validation, case-sensitive matching, reference separator normalization, and tool-prefix handling.Update native smoke callers to supply temporary allowlists, make the tools generator require the existing shared source without losing helper history, retire the obsolete snapshot synchronization workflow, and update tests and documentation.
Validation:
telemetry-reporter/,dotnet build .\ghcfa-telem.slnx --configuration Releasepassed with no warnings or errors.telemetry-reporter/,dotnet test .\ghcfa-telem.slnx --configuration Release --no-buildpassed: 149 tests.npm --prefix scripts run typecheckpassed.npm --prefix scripts run lint -- src/generate-azure-mcp-allowlist.ts src/__tests__/generate-azure-mcp-allowlist.test.ts src/__tests__/telemetry-packaging.test.tspassed.npm --prefix scripts test -- src/__tests__/generate-azure-mcp-allowlist.test.ts src/__tests__/azure-mcp-publication.test.ts src/__tests__/plugin-allowlists.test.ts src/__tests__/telemetry-packaging.test.tspassed: 141 tests.npm --prefix scripts test -- src/__tests__/generate-azure-mcp-allowlist.test.ts src/__tests__/azure-mcp-publication.test.ts src/__tests__/plugin-allowlists.test.ts src/__tests__/telemetry-hooks.test.ts src/__tests__/telemetry-packaging.test.tsinitially reported eight failures: five packaging tests passed after restoring missing root dependencies; three tests in unchanged Windows PowerShell installer code still fail becauseExtractToDirectoryhas no matching three-argument overload.git diff --checkpassed.cd tests && npm testwas not run; this change targets the standalone reporter and script tooling, not skills. Skill descriptions are unchanged, so routing integration tests are not applicable.Checklist
cd tests && npm test)fix:,feat:,feature:,chore:,misc:,test:,eval:tests/,npm run test:vally -- --plugin <plugin-dirname> --skill <skill>)Related Issues
Fixes #3455