Fix write-to-read codemod incorrectly converting id-token and copilot-requests permissions (#25604)

Author: Copilot

pkg/cli/codemod_permissions_test.go

@@ -428,3 +428,124 @@ This workflow needs permissions.`
 	assert.Contains(t, result, "# Test Workflow")
 	assert.Contains(t, result, "This workflow needs permissions.")
 }
+
+func TestWritePermissionsCodemod_SkipsIdToken(t *testing.T) {
+	codemod := getMigrateWritePermissionsToReadCodemod()
+
+	content := `---
+on: workflow_dispatch
+permissions:
+  contents: read
+  id-token: write
+---
+
+# Test`
+
+	frontmatter := map[string]any{
+		"on": "workflow_dispatch",
+		"permissions": map[string]any{
+			"contents": "read",
+			"id-token": "write",
+		},
+	}
+
+	result, applied, err := codemod.Apply(content, frontmatter)
+
+	require.NoError(t, err, "codemod should not return an error")
+	assert.False(t, applied, "Should not be applied when only write-only permissions have write")
+	assert.Equal(t, content, result, "codemod result should be unchanged when only write-only permissions have write")
+	assert.Contains(t, result, "id-token: write", "id-token permission should remain write after codemod")
+	assert.NotContains(t, result, "id-token: read", "id-token should never be converted to read")
+}
+
+func TestWritePermissionsCodemod_SkipsCopilotRequests(t *testing.T) {
+	codemod := getMigrateWritePermissionsToReadCodemod()
+
+	content := `---
+on: workflow_dispatch
+permissions:
+  contents: read
+  copilot-requests: write
+---
+
+# Test`
+
+	frontmatter := map[string]any{
+		"on": "workflow_dispatch",
+		"permissions": map[string]any{
+			"contents":         "read",
+			"copilot-requests": "write",
+		},
+	}
+
+	result, applied, err := codemod.Apply(content, frontmatter)
+
+	require.NoError(t, err, "codemod should not return an error")
+	assert.False(t, applied, "Should not be applied when only write-only permissions have write")
+	assert.Equal(t, content, result, "codemod result should be unchanged when only write-only permissions have write")
+	assert.Contains(t, result, "copilot-requests: write", "copilot-requests permission should remain write after codemod")
+	assert.NotContains(t, result, "copilot-requests: read", "copilot-requests should never be converted to read")
+}
+
+func TestWritePermissionsCodemod_MixedWithIdToken(t *testing.T) {
+	codemod := getMigrateWritePermissionsToReadCodemod()
+
+	content := `---
+on: workflow_dispatch
+permissions:
+  contents: write
+  issues: write
+  id-token: write
+---
+
+# Test`
+
+	frontmatter := map[string]any{
+		"on": "workflow_dispatch",
+		"permissions": map[string]any{
+			"contents": "write",
+			"issues":   "write",
+			"id-token": "write",
+		},
+	}
+
+	result, applied, err := codemod.Apply(content, frontmatter)
+
+	require.NoError(t, err, "codemod should not return an error")
+	assert.True(t, applied, "codemod should be applied when non-write-only permissions have write")
+	assert.Contains(t, result, "contents: read", "contents permission should be downgraded from write to read")
+	assert.Contains(t, result, "issues: read", "issues permission should be downgraded from write to read")
+	// id-token must remain write — "read" is not a valid value for it
+	assert.Contains(t, result, "id-token: write", "id-token permission should remain write after codemod")
+	assert.NotContains(t, result, "id-token: read", "id-token should never be converted to read")
+}
+
+func TestWritePermissionsCodemod_MixedWithCopilotRequests(t *testing.T) {
+	codemod := getMigrateWritePermissionsToReadCodemod()
+
+	content := `---
+on: workflow_dispatch
+permissions:
+  contents: write
+  copilot-requests: write
+---
+
+# Test`
+
+	frontmatter := map[string]any{
+		"on": "workflow_dispatch",
+		"permissions": map[string]any{
+			"contents":         "write",
+			"copilot-requests": "write",
+		},
+	}
+
+	result, applied, err := codemod.Apply(content, frontmatter)
+
+	require.NoError(t, err, "codemod should not return an error")
+	assert.True(t, applied, "codemod should be applied when non-write-only permissions have write")
+	assert.Contains(t, result, "contents: read", "contents permission should be downgraded from write to read")
+	// copilot-requests must remain write — "read" is not a valid value for it
+	assert.Contains(t, result, "copilot-requests: write", "copilot-requests permission should remain write after codemod")
+	assert.NotContains(t, result, "copilot-requests: read", "copilot-requests should never be converted to read")
+}

pkg/cli/codemod_permissions_write.go

@@ -9,6 +9,13 @@ import (
 
 var writePermissionsCodemodLog = logger.New("cli:codemod_permissions")
 
+// writeOnlyPermissions are permission scopes that only accept "write" or "none" as valid values.
+// These must never be converted to "read" since "read" is not a valid value for them.
+var writeOnlyPermissions = map[string]bool{
+	"id-token":         true, // OIDC token: only write or none
+	"copilot-requests": true, // Copilot authentication token: only write or none
+}
+
 // getMigrateWritePermissionsToReadCodemod creates a codemod for converting write permissions to read
 func getMigrateWritePermissionsToReadCodemod() Codemod {
 	return Codemod{
@@ -35,7 +42,12 @@ func getMigrateWritePermissionsToReadCodemod() Codemod {
 
 			// Handle map format
 			if mapValue, ok := permissionsValue.(map[string]any); ok {
-				for _, value := range mapValue {
+				for key, value := range mapValue {
+					// Skip write-only permissions (e.g. id-token, copilot-requests) since
+					// "read" is not a valid value for them — they only accept "write" or "none"
+					if writeOnlyPermissions[key] {
+						continue
+					}
 					if strValue, ok := value.(string); ok && strValue == "write" {
 						hasWritePermissions = true
 						break
@@ -91,8 +103,17 @@ func getMigrateWritePermissionsToReadCodemod() Codemod {
 						parts := strings.SplitN(line, ":", 2)
 						if len(parts) >= 2 {
 							key := parts[0]
+							permKey := strings.TrimSpace(key)
 							valueAndComment := parts[1]
 
+							// Skip write-only permissions (e.g. id-token, copilot-requests) since
+							// "read" is not a valid value for them — they only accept "write" or "none"
+							if writeOnlyPermissions[permKey] {
+								result[i] = line
+								writePermissionsCodemodLog.Printf("Skipping write-only permission %q on line %d", permKey, i+1)
+								continue
+							}
+
 							// Replace "write" with "read" in the value part
 							newValueAndComment := strings.Replace(valueAndComment, " write", " read", 1)
 							result[i] = fmt.Sprintf("%s:%s", key, newValueAndComment)