Skip to content

Commit 405aeb4

Browse files
williammartinCopilot
williammartin
and
Copilot
committed
Record official extension telemetry
Third-party extensions have names that can be user-authored identifiers (organization names, project names, etc.), so historically all extension commands have had telemetry disabled via cmdutil.DisableTelemetry. This change allows GitHub-owned 'official' extensions (listed in a hard-coded OfficialExtensions registry) to report telemetry, because their command names come from that fixed registry and cannot contain sensitive user identifiers. Only the extension name is checked: cmdutil.RecordTelemetry records cmd.CommandPath() and parsed flag names. Extension commands set DisableFlagParsing, so no flags are ever recorded, and the command name is always either a stub name registered directly from the registry or the filename of an installed gh-<name> binary. Owner and host therefore never reach telemetry and are not part of the gate. Comparison is case-insensitive because extension names are derived from filenames and preserve install-time casing. Co-authored-by: Copilot <[email protected]>
1 parent c788fd9 commit 405aeb4

6 files changed

Lines changed: 131 additions & 2 deletions

File tree

‎acceptance/testdata/telemetry/no-telemetry-for-extension.txtar‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
# Extensions should not generate telemetry events
1+
# Third-party extensions must not generate telemetry events, since the
2+
# extension command name can be a user-authored identifier (e.g. an
3+
# organization or project name).
24
[!exec:bash] skip
35

46
env GH_PRIVATE_ENABLE_TELEMETRY=1

‎acceptance/testdata/telemetry/telemetry-for-official-extension-stub.txtar‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Official extension stubs (the hidden commands suggesting installation of
2+
# GitHub-owned extensions) are safe to report via telemetry: their command
3+
# names come from a fixed, hard-coded registry and do not contain any
4+
# user-authored identifiers.
5+
6+
env GH_PRIVATE_ENABLE_TELEMETRY=1
7+
env GH_TELEMETRY=log
8+
env GH_TELEMETRY_SAMPLE_RATE=100
9+
10+
# `stack` is registered in extensions.OfficialExtensions. Since no real
11+
# extension is installed, the hidden stub runs and, in a non-TTY session,
12+
# prints install instructions without prompting.
13+
exec gh stack
14+
stderr 'gh extension install github/gh-stack'
15+
16+
# The stub invocation records a command_invocation event for the stub's
17+
# command path.
18+
stderr 'Telemetry payload:'
19+
stderr '"type": "command_invocation"'
20+
stderr '"command": "gh stack"'

‎pkg/cmd/root/extension.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,14 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
7979
}
8080

8181
cmdutil.DisableAuthCheck(cmd)
82-
cmdutil.DisableTelemetry(cmd)
82+
// Extensions are user-installed and their names can be arbitrary
83+
// (potentially including sensitive identifiers such as project or
84+
// organization names), so we must not record telemetry for them by
85+
// default. Official GitHub-owned extensions are a known, fixed set and
86+
// can safely contribute their command name to telemetry.
87+
if !extensions.IsOfficial(ext.Name()) {
88+
cmdutil.DisableTelemetry(cmd)
89+
}
8390

8491
return cmd
8592
}

‎pkg/cmd/root/extension_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,3 +234,43 @@ func TestNewCmdExtension_UpdateCheckIsNonblocking(t *testing.T) {
234234
t.Fatal("extension update check should have exited")
235235
}
236236
}
237+
238+
func TestNewCmdExtension_TelemetryEnabledForOfficialExtensions(t *testing.T) {
239+
tests := []struct {
240+
name string
241+
extName string
242+
wantTelemetryOff bool
243+
}{
244+
{
245+
name: "official extension records telemetry",
246+
extName: "stack",
247+
wantTelemetryOff: false,
248+
},
249+
{
250+
name: "official extension name with mixed case still records telemetry",
251+
extName: "STACK",
252+
wantTelemetryOff: false,
253+
},
254+
{
255+
name: "third-party extension disables telemetry",
256+
extName: "my-custom-ext",
257+
wantTelemetryOff: true,
258+
},
259+
}
260+
261+
for _, tt := range tests {
262+
t.Run(tt.name, func(t *testing.T) {
263+
ios, _, _, _ := iostreams.Test()
264+
em := &extensions.ExtensionManagerMock{}
265+
ext := &extensions.ExtensionMock{
266+
NameFunc: func() string { return tt.extName },
267+
}
268+
269+
cmd := root.NewCmdExtension(ios, em, ext, func(extensions.ExtensionManager, extensions.Extension) (*update.ReleaseInfo, error) {
270+
return nil, nil
271+
})
272+
273+
assert.Equal(t, tt.wantTelemetryOff, cmd.Annotations["telemetry"] == "disabled")
274+
})
275+
}
276+
}

‎pkg/extensions/official.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package extensions
22

33
import (
4+
"strings"
5+
46
"github.com/cli/cli/v2/internal/ghrepo"
57
)
68

@@ -24,3 +26,26 @@ var OfficialExtensions = []OfficialExtension{
2426
{Name: "aw", Owner: "github", Repo: "gh-aw"},
2527
{Name: "stack", Owner: "github", Repo: "gh-stack"},
2628
}
29+
30+
// IsOfficial reports whether the given extension command name matches an
31+
// entry in the OfficialExtensions registry. Only the name is checked
32+
// because the name is the only value that can reach telemetry:
33+
// cmdutil.RecordTelemetry records cmd.CommandPath() (plus parsed flag
34+
// names, which for extensions are empty because DisableFlagParsing is set).
35+
// A user running `gh <name>` therefore only emits telemetry for <name>
36+
// when the registered cobra command carries that string, which is either
37+
// a hard-coded stub name from this registry or the filename of an
38+
// installed `gh-<name>` binary. Owner and host never reach telemetry, so
39+
// they are not part of the check.
40+
//
41+
// Comparison is case-insensitive because extension names come from
42+
// filenames and installed extensions preserve whatever casing was used at
43+
// install time.
44+
func IsOfficial(name string) bool {
45+
for _, ext := range OfficialExtensions {
46+
if strings.EqualFold(ext.Name, name) {
47+
return true
48+
}
49+
}
50+
return false
51+
}

‎pkg/extensions/official_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,38 @@ func TestOfficialExtension_Repository(t *testing.T) {
1313
assert.Equal(t, "gh-stack", repo.RepoName())
1414
assert.Equal(t, "github.com", repo.RepoHost())
1515
}
16+
17+
func TestIsOfficial(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
extName string
21+
want bool
22+
}{
23+
{
24+
name: "known official extension matches",
25+
extName: "stack",
26+
want: true,
27+
},
28+
{
29+
name: "mixed-case name still matches",
30+
extName: "STACK",
31+
want: true,
32+
},
33+
{
34+
name: "unknown name is not official",
35+
extName: "not-a-real-extension",
36+
want: false,
37+
},
38+
{
39+
name: "empty name is not official",
40+
extName: "",
41+
want: false,
42+
},
43+
}
44+
45+
for _, tt := range tests {
46+
t.Run(tt.name, func(t *testing.T) {
47+
assert.Equal(t, tt.want, IsOfficial(tt.extName))
48+
})
49+
}
50+
}

0 commit comments

Comments
 (0)