Fix GitOps dry run issue with validating profiles with secrets (#32104)
Fixes #31477 Docs PR: https://github.com/fleetdm/fleet/pull/32116 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - GitOps now supports FLEET_SECRET_ placeholders in macOS (.mobileconfig/.xml) profiles. Secrets are expanded only for validation, while remaining unexpanded in uploaded content. - Improved environment variable handling: non-secret vars expand as before; server-side secrets are preserved. - Validation enforces that profile display names cannot contain FLEET_SECRET_ values. - Bug Fixes - Resolves validation issues when FLEET_SECRET_ appears in <data> tags by performing safe client-side expansion for validation. - More accurate error reporting during profile parsing and validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Lucas Manuel Rodriguez <lucas@fleetdm.com>
This commit is contained in:
Victor Lyuboslavsky
co-authored by
Lucas Manuel Rodriguez
Lucas Manuel Rodriguez
parent
53b7a0628a
commit
1d7aab04ab
@@ -1270,3 +1270,240 @@ queries:
|
||||
require.Equal(t, secretVariables[0].Name, secretName)
|
||||
require.Equal(t, secretVariables[0].Value, secretValue)
|
||||
}
|
||||
|
||||
// TestEnvSubstitutionInProfiles tests that only FLEET_SECRET_ prefixed env vars are saved as secrets
|
||||
func (s *enterpriseIntegrationGitopsTestSuite) TestEnvSubstitutionInProfiles() {
|
||||
t := s.T()
|
||||
ctx := t.Context()
|
||||
tempDir := t.TempDir()
|
||||
|
||||
// Create a test configuration profile with both valid and invalid secret references
|
||||
profileContent := `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
|
||||
<plist version="1.0">
|
||||
<dict>
|
||||
<key>PayloadContent</key>
|
||||
<array>
|
||||
<dict>
|
||||
<key>PayloadDisplayName</key>
|
||||
<string>Test Profile</string>
|
||||
<key>PayloadIdentifier</key>
|
||||
<string>com.fleet.test.env</string>
|
||||
<key>PayloadType</key>
|
||||
<string>Configuration</string>
|
||||
<key>PayloadUUID</key>
|
||||
<string>12345678-1234-1234-1234-123456789012</string>
|
||||
<key>PayloadVersion</key>
|
||||
<integer>1</integer>
|
||||
<key>TestSecretValue</key>
|
||||
<string>$FLEET_SECRET_TEST_SECRET</string>
|
||||
<key>TestInvalidSecret</key>
|
||||
<string>$FLEET_DUO_CERTIFICATE_SECRET</string>
|
||||
<key>TestPlainValue</key>
|
||||
<string>$HOME</string>
|
||||
</dict>
|
||||
</array>
|
||||
<key>PayloadDisplayName</key>
|
||||
<string>Test Profile</string>
|
||||
<key>PayloadIdentifier</key>
|
||||
<string>com.fleet.test.env</string>
|
||||
<key>PayloadType</key>
|
||||
<string>Configuration</string>
|
||||
<key>PayloadUUID</key>
|
||||
<string>12345678-1234-1234-1234-123456789012</string>
|
||||
<key>PayloadVersion</key>
|
||||
<integer>1</integer>
|
||||
</dict>
|
||||
</plist>`
|
||||
|
||||
// Write the profile to a file
|
||||
profilePath := filepath.Join(tempDir, "test-profile.mobileconfig")
|
||||
err := os.WriteFile(profilePath, []byte(profileContent), 0644) //nolint:gosec // test code
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a GitOps config file that references the profile
|
||||
// Note: Environment variables in the YAML config itself get expanded,
|
||||
// but not in the referenced profile files
|
||||
gitopsConfig := fmt.Sprintf(`
|
||||
org_settings:
|
||||
server_settings:
|
||||
server_url: %s
|
||||
secrets:
|
||||
- secret: test_secret
|
||||
agent_options:
|
||||
config:
|
||||
decorators:
|
||||
load:
|
||||
- SELECT uuid AS host_uuid FROM system_info;
|
||||
controls:
|
||||
macos_settings:
|
||||
custom_settings:
|
||||
- path: %s
|
||||
queries: []
|
||||
policies: []
|
||||
`, s.Server.URL, profilePath)
|
||||
|
||||
configPath := filepath.Join(tempDir, "gitops.yml")
|
||||
err = os.WriteFile(configPath, []byte(gitopsConfig), 0644) //nolint:gosec // test code
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a GitOps user
|
||||
gitOpsUser := s.createGitOpsUser(t)
|
||||
fleetctlConfig := s.createFleetctlConfig(t, gitOpsUser)
|
||||
|
||||
// Set the environment variable for the valid secret
|
||||
t.Setenv("FLEET_SECRET_TEST_SECRET", "super_secret_value_123")
|
||||
t.Setenv("FLEET_DUO_CERTIFICATE_SECRET", "should_not_be_saved")
|
||||
t.Setenv("HOME", "also_not_saved")
|
||||
|
||||
// Run GitOps dry-run - should fail without the required secret
|
||||
// First, unset the environment variable to trigger the error
|
||||
_ = os.Unsetenv("FLEET_SECRET_TEST_SECRET")
|
||||
_, err = fleetctl.RunAppNoChecks([]string{"gitops", "--config", fleetctlConfig.Name(), "-f", configPath, "--dry-run"})
|
||||
require.ErrorContains(t, err, "FLEET_SECRET_TEST_SECRET")
|
||||
|
||||
// Set the env var again and run for real
|
||||
t.Setenv("FLEET_SECRET_TEST_SECRET", "super_secret_value_123")
|
||||
_ = fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetctlConfig.Name(), "-f", configPath})
|
||||
|
||||
// Verify that the secret was saved to the server
|
||||
secrets, err := s.DS.GetSecretVariables(ctx, []string{"TEST_SECRET"})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, secrets, 1)
|
||||
assert.Equal(t, "TEST_SECRET", secrets[0].Name)
|
||||
assert.Equal(t, "super_secret_value_123", secrets[0].Value)
|
||||
|
||||
// Verify that non-FLEET_SECRET_ variables were NOT saved
|
||||
notSaved, err := s.DS.GetSecretVariables(ctx, []string{"DUO_CERTIFICATE_SECRET", "HOME"})
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, notSaved, "Non-FLEET_SECRET_ variables should not be saved")
|
||||
|
||||
// Verify that the profile content has the expected substitutions:
|
||||
// - $FLEET_SECRET_* variables should remain as-is (substituted at delivery time)
|
||||
// - Other env vars should be expanded during GitOps
|
||||
profiles, err := s.DS.ListMDMAppleConfigProfiles(ctx, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
foundProfile := false
|
||||
for _, profile := range profiles {
|
||||
t.Logf("Found profile: %s", profile.Name)
|
||||
if strings.Contains(profile.Name, "test-profile") || strings.Contains(profile.Identifier, "com.fleet.test.env") {
|
||||
foundProfile = true
|
||||
// $FLEET_SECRET_* variables should NOT be expanded (they're expanded at delivery time)
|
||||
assert.Contains(t, string(profile.Mobileconfig), "$FLEET_SECRET_TEST_SECRET")
|
||||
// Non-FLEET_SECRET_/FLEET_VAR_ variables SHOULD be expanded during GitOps
|
||||
assert.Contains(t, string(profile.Mobileconfig), "should_not_be_saved") // Value of $FLEET_DUO_CERTIFICATE_SECRET
|
||||
assert.Contains(t, string(profile.Mobileconfig), "also_not_saved") // Value of $HOME
|
||||
// The original variable names should NOT be present
|
||||
assert.NotContains(t, string(profile.Mobileconfig), "$FLEET_DUO_CERTIFICATE_SECRET")
|
||||
assert.NotContains(t, string(profile.Mobileconfig), "$HOME")
|
||||
break
|
||||
}
|
||||
}
|
||||
assert.True(t, foundProfile, "Profile should be uploaded to the server")
|
||||
}
|
||||
|
||||
// TestFleetSecretInDataTag tests that FLEET_SECRET_ variables in <data> tags of Apple profiles
|
||||
// are handled properly.
|
||||
func (s *enterpriseIntegrationGitopsTestSuite) TestFleetSecretInDataTag() {
|
||||
t := s.T()
|
||||
tempDir := t.TempDir()
|
||||
ctx := context.Background()
|
||||
|
||||
// Sample certificate in base64 format (this is a dummy test certificate)
|
||||
testCertBase64 := `MIIDaTCCAlGgAwIBAgIUNQLezMUpmZK18DcLKt/XTRcLlK8wDQYJKoZIhvcNAQELBQAwRDEfMB0GA1UEAwwWRHVtbXkgVGVzdCBDZXJ0aWZpY2F0ZTEUMBIGA1UECgwLRXhhbXBsZSBPcmcxCzAJBgNVBAYTAlVTMB4XDTI1MDgxOTE3NDMwN1oXDTI2MDgxOTE3NDMwN1owRDEfMB0GA1UEAwwWRHVtbXkgVGVzdCBDZXJ0aWZpY2F0ZTEUMBIGA1UECgwLRXhhbXBsZSBPcmcxCzAJBgNVBAYTAlVTMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA07Np/w5WpFVLlMKX3dZSxwo+c2uwP2glTN0HA5c/6UOQRR9c91yoGGJsD4pfqhtIMSTFw7po3n/PjhGDe/WH+utK+ZIcD0nGD6SvmOggyoohHs81eIOjJAEJjxzhk7eLTVpUI2EnPe/24ei/dgkK59As9qQyH/y+CoR8JIYbNCJH5YLC2Pa44V84QWa2I5DHKUKrUXo9WsrRp1N1JjyaG/6hxLBJZ69e0QTrxxScboreRqVUR6oIEJRTchB+rDG5dxXzCQE6/F8N3qR76t23wd3CLmrcXoEc1P2P331Qzi0KXNXjdJFf0plmfRkT/IWgfM81Vfon1QwENwRSBNmPfQIDAQABo1MwUTAdBgNVHQ4EFgQU9q7SDfQRbJ31snRt2sZzx5sdEpYwHwYDVR0jBBgwFoAU9q7SDfQRbJ31snRt2sZzx5sdEpYwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAYwH42JP45SnZejSF74OcYt8fp08jCWHOiFC3QEo3cROXVn6AWjEbzuQpOxRWF9EizNA83c4E6I+kQVztiuv9bUKGuLFeYb9lUZe8HOvH+j22MtGvZrDPygsJc8TavdkxAsu6OiNQZrYiCFzixkKS9b5p/1B93GBh62OFnV1nUBS8PzAZhOAyJ8UcEhr+GNzZG99/wOkcB0uwxmIb8x8sB3KnQ0qef/qnmgeWxlJlDc/SZ2/4PgtaluZ+noDfNPzaQn4eJNnBz0OTqZ9yuKALeE1WHk8U13zSdc1GNVLhXOrEHegPK5bBmA/lpIQ6HrkwUX7MJ3vK0AD3LjaTzXltDQ==`
|
||||
|
||||
// Create a test team first
|
||||
team, err := s.DS.NewTeam(ctx, &fleet.Team{
|
||||
Name: "Test Team for Secret in Data Tag",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a profile with $FLEET_SECRET_DUO_CERTIFICATE in a <data> tag
|
||||
// This mimics the real-world scenario where the certificate should be base64 encoded
|
||||
profileContent := `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
|
||||
<plist version="1.0">
|
||||
<dict>
|
||||
<key>PayloadContent</key>
|
||||
<array>
|
||||
<dict>
|
||||
<key>PayloadType</key>
|
||||
<string>com.apple.security.root</string>
|
||||
<key>PayloadVersion</key>
|
||||
<integer>1</integer>
|
||||
<key>PayloadIdentifier</key>
|
||||
<string>com.example.test.cert</string>
|
||||
<key>PayloadUUID</key>
|
||||
<string>11111111-2222-3333-4444-555555555555</string>
|
||||
<key>PayloadDisplayName</key>
|
||||
<string>Test Root Certificate</string>
|
||||
<key>PayloadContent</key>
|
||||
<data>$FLEET_SECRET_DUO_CERTIFICATE</data>
|
||||
</dict>
|
||||
</array>
|
||||
<key>PayloadType</key>
|
||||
<string>Configuration</string>
|
||||
<key>PayloadVersion</key>
|
||||
<integer>1</integer>
|
||||
<key>PayloadIdentifier</key>
|
||||
<string>com.example.test.profile</string>
|
||||
<key>PayloadUUID</key>
|
||||
<string>aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee</string>
|
||||
<key>PayloadDisplayName</key>
|
||||
<string>Test MDM Profile with Base64</string>
|
||||
</dict>
|
||||
</plist>`
|
||||
|
||||
// Write the profile to a file
|
||||
profilePath := filepath.Join(tempDir, "rootcert-secret.mobileconfig")
|
||||
err = os.WriteFile(profilePath, []byte(profileContent), 0644) //nolint:gosec
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a team GitOps config file that references the profile
|
||||
teamConfig := fmt.Sprintf(`
|
||||
name: %s
|
||||
team_settings:
|
||||
secrets:
|
||||
- secret: test_secret
|
||||
agent_options:
|
||||
config:
|
||||
decorators:
|
||||
load:
|
||||
- SELECT uuid AS host_uuid FROM system_info;
|
||||
controls:
|
||||
macos_settings:
|
||||
custom_settings:
|
||||
- path: %s
|
||||
queries:
|
||||
policies:
|
||||
software:
|
||||
`, team.Name, profilePath)
|
||||
|
||||
configPath := filepath.Join(tempDir, "team-gitops.yml")
|
||||
err = os.WriteFile(configPath, []byte(teamConfig), 0644) //nolint:gosec
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a GitOps user
|
||||
gitOpsUser := s.createGitOpsUser(t)
|
||||
fleetctlConfig := s.createFleetctlConfig(t, gitOpsUser)
|
||||
|
||||
// Set the environment variable with the base64-encoded certificate
|
||||
t.Setenv("FLEET_SECRET_DUO_CERTIFICATE", testCertBase64)
|
||||
|
||||
// The fix expands FLEET_SECRET_ variables for validation only, allowing the profile to be parsed
|
||||
_, err = fleetctl.RunAppNoChecks([]string{"gitops", "--config", fleetctlConfig.Name(), "-f", configPath, "--dry-run"})
|
||||
require.NoError(t, err, "GitOps dry-run should succeed with the fix")
|
||||
|
||||
// Also test without dry-run to confirm it works
|
||||
_, err = fleetctl.RunAppNoChecks([]string{"gitops", "--config", fleetctlConfig.Name(), "-f", configPath})
|
||||
require.NoError(t, err, "GitOps should succeed with the fix")
|
||||
|
||||
// Verify that the profile stored on the server still has the unexpanded variable
|
||||
profiles, err := s.DS.ListMDMAppleConfigProfiles(ctx, &team.ID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, profiles, 1)
|
||||
// The stored profile should still contain the unexpanded variable, not the actual secret
|
||||
require.Contains(t, string(profiles[0].Mobileconfig), "$FLEET_SECRET_DUO_CERTIFICATE",
|
||||
"Profile should still contain unexpanded FLEET_SECRET variable")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user