From b6db54e3951c6aee46b83a03e8e1efca5703ce50 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Tue, 14 Apr 2026 22:36:56 +0000 Subject: [PATCH 01/18] rcv2 loads policy file into policy object --- go.mod | 2 +- go.sum | 2 + internal/cmds/cmds.go | 30 +++++++++++ internal/constants/constants.go | 3 ++ internal/constants/exitcodes.go | 9 +++- internal/types/extensionpolicysettings.go | 63 +++++++++++++++++++++++ misc/HandlerManifest.json | 1 + 7 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 internal/types/extensionpolicysettings.go diff --git a/go.mod b/go.mod index f76efeb..227dbb3 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ toolchain go1.24.5 require ( github.com/Azure/azure-extension-foundation v0.0.0-20250620154556-caff9e3c3c5c - github.com/Azure/azure-extension-platform v0.0.0-20250107200156-aa20f765d49f + github.com/Azure/azure-extension-platform v0.0.0-20260410171604-91b4725acbb1 github.com/Azure/azure-sdk-for-go v68.0.0+incompatible github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.0 diff --git a/go.sum b/go.sum index 6ee1b20..12af36b 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/Azure/azure-extension-platform v0.0.0-20240610175536-404c704f82f8 h1: github.com/Azure/azure-extension-platform v0.0.0-20240610175536-404c704f82f8/go.mod h1:nEQQIC3RKmMnpdc+RakYHIdu556jdcHv67ML8PdsQeQ= github.com/Azure/azure-extension-platform v0.0.0-20250107200156-aa20f765d49f h1:ddsUz/suc9txCMz/xWOslqNMvzhbWFMTflUrbcMNoSw= github.com/Azure/azure-extension-platform v0.0.0-20250107200156-aa20f765d49f/go.mod h1:0458BvQsi5ch6kn+KZtI5m88Z3L9UFXdoY1+6nKdivY= +github.com/Azure/azure-extension-platform v0.0.0-20260410171604-91b4725acbb1 h1:ijfz4hQtWTfTmaejzDrkNqhpCfOGFzKk/wUtPZmLrg0= +github.com/Azure/azure-extension-platform v0.0.0-20260410171604-91b4725acbb1/go.mod h1:0458BvQsi5ch6kn+KZtI5m88Z3L9UFXdoY1+6nKdivY= github.com/Azure/azure-sdk-for-go v68.0.0+incompatible h1:fcYLmCpyNYRnvJbPerq7U0hS+6+I79yEDJBqVNcqUzU= github.com/Azure/azure-sdk-for-go v68.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 h1:JZg6HRh6W6U4OLl6lk7BZ7BLisIzM9dG1R50zUk9C/M= diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 57c731c..4e9dd62 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -18,6 +18,7 @@ import ( "time" "github.com/Azure/azure-extension-platform/pkg/extensionevents" + "github.com/Azure/azure-extension-platform/pkg/extensionpolicysettings" "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" "github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming" @@ -210,6 +211,35 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma return "", "", err, exitCode } + // Load extension policy settings. + // If policy file exists, load the policy. If not, then don't load. + var ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings] + policyPath := filepath.Join(h.HandlerEnvironment.ConfigFolder, constants.PolicyFileName) + + if _, err := os.Stat(policyPath); err == nil { + ExtensionPolicyManagerPtr, err = extensionpolicysettings.NewExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings](policyPath) + if err != nil { + return "", "", errors.Wrap(err, "failed to create extension policy settings manager"), constants.ExitCode_LoadExtensionPolicySettingsFailed + } + + err = ExtensionPolicyManagerPtr.LoadExtensionPolicySettings() + if err != nil { + return "", "", errors.Wrap(err, "failed to load extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed + } else { + settings, err := ExtensionPolicyManagerPtr.GetSettings() + + if err != nil { + return "", "", errors.Wrap(err, "failed to get extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed + } + ctx.Log("message", "successfully loaded extension policy settings", "settings", settings) + } + } else if !os.IsNotExist(err) { + ctx.Log("message", "extension policy settings file does not exist. No policy applied.", "error", err) + ExtensionPolicyManagerPtr = nil + } else { + return "", "", errors.Wrap(err, "failed to stat extension policy settings file"), constants.ExitCode_LoadExtensionPolicySettingsFailed + } + dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) scriptFilePath, err := downloadScript(ctx, dir, &cfg) if err != nil { diff --git a/internal/constants/constants.go b/internal/constants/constants.go index fe7f0da..b7aacbf 100755 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -77,4 +77,7 @@ const ( // The name of the file that contains the immediate goal states that reached the terminal status ImmediateGoalStatesInTerminalStatusFileName = "immediateGoalStatesInTerminalStatusFile.status" + + // The name of the policy file the extension uses to validate the command + scripts before execution. + PolicyFileName = "waagent_runtime_policy.json" ) diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index 7414ec6..2ce23df 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -36,5 +36,12 @@ const ( ExitCode_ImmediateTaskFailed = -223 ExitCode_CouldNotRehydrateMrSeq = -224 - // Unknown errors (-300s): + // Extension Policy Settings Errors (-300s): + ExitCode_LoadExtensionPolicySettingsFailed = -300 + ExitCode_GetExtensionPolicySettingsFailed = -301 + ExitCode_ExtensionPolicyInvalid = -302 + ExitCode_CommandNotAllowedByPolicy = -303 + ExitCode_ScriptNotAllowedByPolicy = -304 + + // Unknown errors (-400s): ) diff --git a/internal/types/extensionpolicysettings.go b/internal/types/extensionpolicysettings.go new file mode 100644 index 0000000..7d24d83 --- /dev/null +++ b/internal/types/extensionpolicysettings.go @@ -0,0 +1,63 @@ +package types + +// ScriptType refers to the type of script being executed in a run command. +// This type defintion matches the ScriptType definition in CRP in the Run Command Handler, +// and should always be kept in sync with the ScriptType definition in RCv2 Windows. +// None is defined in the case where no script is passed down, which is a valid scenario. +type ScriptType string + +const ( + InlineScript ScriptType = "inline" + DownloadedScript ScriptType = "downloaded" + GalleryScript ScriptType = "gallery" + DiagnosticScript ScriptType = "diagnostic" + CommandIdScript ScriptType = "commandId" + NoneScript ScriptType = "none" +) + +// This refers *specifically* to file types that require signature verification +// when RequireSigning is enabled for RCv2. This is not a general enum for all file types in the extension. +// Non-script file types include binaries, parameter files, etc. +type FileType string + +const ( + All FileType = "all" + NoFiles FileType = "none" // Named NoFiles instead of None to avoid conflict with ScriptType.None below. + Scripts FileType = "scripts" +) + +// AllowedScriptType is a bitmask enum that defines which types of scripts run command is +// allowed to execute based on customer policy. This should always match the AllowedScriptType in RCv2 Windows. +type AllowedScriptType int + +const ( + AllowedCommandId int = 1 << iota + Gallery + Diagnostic + Inline + AllowedDownloaded + AllowAll = AllowedCommandId | Gallery | Diagnostic | Inline | AllowedDownloaded +) + +// RCv2ExtensionPolicySettings defines the structure of the policy file for RCv2. +// RequireSigning: describes the types of files that require signature verification. +// FileRootCert: the root certificate used for signature verification. Required if RequireSigning is not "none". +// DownloadedScriptsAllowlist: if scripts are limited to a specific allowlist, this is the list of hashes of the allowed scripts. +// CommandIdAllowlist: if commandId scripts are allowed only from specific commandIds, this is the list of allowed commandIds. +// RunAsUser: the only user with permission to run scripts. If another user tries to run a script, the command will fail. +// LimitScripts: the types of scripts that are allowed to be executed. +type RCv2ExtensionPolicySettings struct { + // RequireSigning FileType `json:"requireSigning"` + // FileRootCert string `json:"fileRootCert,omitempty"` + DownloadedScriptsAllowlist []string `json:"downloadedScriptsAllowlist,omitempty"` + CommandIdAllowlist []string `json:"commandIdAllowlist,omitempty"` + RunAsUser string `json:"runAsUser,omitempty"` + LimitScripts AllowedScriptType `json:"limitScripts,omitempty"` + DisableOutputBlobs bool `json:"disableOutputBlobs,omitempty"` +} + +// This function is called from within the LoadExtensionPolicySettings function in extensionpolicysettings.go +// to validate the format of our policy. +func (rceps RCv2ExtensionPolicySettings) ValidateFormat() error { + return nil +} diff --git a/misc/HandlerManifest.json b/misc/HandlerManifest.json index f5b1e52..cce9c66 100644 --- a/misc/HandlerManifest.json +++ b/misc/HandlerManifest.json @@ -6,6 +6,7 @@ "updateCommand": "bin/run-command-shim update", "enableCommand": "bin/run-command-shim enable", "disableCommand": "bin/run-command-shim disable", + "supportsPolicy": true, "rebootAfterInstall": false, "reportHeartbeat": false, "updateMode": "UpdateWithInstall", From af1978e56c844f1fea363900abade3062f658d36 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Thu, 16 Apr 2026 22:30:12 +0000 Subject: [PATCH 02/18] onboarded rcv2 to extension policy settings, initial limit of scripts based on type --- internal/cmds/cmds.go | 22 ++++- internal/constants/exitcodes.go | 2 +- internal/handlersettings/types.go | 17 +++- internal/types/extensionpolicysettings.go | 99 +++++++++++++++++++++-- 4 files changed, 126 insertions(+), 14 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 4e9dd62..0637d7f 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -215,6 +215,7 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma // If policy file exists, load the policy. If not, then don't load. var ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings] policyPath := filepath.Join(h.HandlerEnvironment.ConfigFolder, constants.PolicyFileName) + var rceps *types.RCv2ExtensionPolicySettings if _, err := os.Stat(policyPath); err == nil { ExtensionPolicyManagerPtr, err = extensionpolicysettings.NewExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings](policyPath) @@ -226,20 +227,33 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma if err != nil { return "", "", errors.Wrap(err, "failed to load extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed } else { - settings, err := ExtensionPolicyManagerPtr.GetSettings() + rceps, err = ExtensionPolicyManagerPtr.GetSettings() if err != nil { return "", "", errors.Wrap(err, "failed to get extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed } - ctx.Log("message", "successfully loaded extension policy settings", "settings", settings) + ctx.Log("message", "successfully loaded extension policy settings", "settings", rceps) } - } else if !os.IsNotExist(err) { + } else if os.IsNotExist(err) { ctx.Log("message", "extension policy settings file does not exist. No policy applied.", "error", err) ExtensionPolicyManagerPtr = nil } else { return "", "", errors.Wrap(err, "failed to stat extension policy settings file"), constants.ExitCode_LoadExtensionPolicySettingsFailed } + // Limit scripts by type before downloading them. + if ExtensionPolicyManagerPtr != nil && rceps != nil { + allowedScriptType, err := types.StringToAllowedScriptTypeFlag(rceps.LimitScripts) + if err != nil { // We should not hit this because we already validte the policy settings earlier. + return "", "", errors.Wrap(err, "failed to parse allowed script types"), constants.ExitCode_ExtensionPolicyInvalid + } + // Compare the script type of the command with the allowed script types in the policy. + err = types.CompareScriptTypeToAllowedScriptType(cfg.ScriptType(), allowedScriptType) + if err != nil { + return "", "", errors.Wrap(err, "script type is not allowed by policy"), constants.ExitCode_ScriptTypeNotAllowedByPolicy + } + } + dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) scriptFilePath, err := downloadScript(ctx, dir, &cfg) if err != nil { @@ -262,6 +276,7 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma blobCreateOrReplaceError := "Error creating AppendBlob '%s' using SAS token or Managed identity. Please use a valid blob SAS URI with [read, append, create, write] permissions OR managed identity. If managed identity is used, make sure Azure blob and identity exist, and identity has been given access to storage blob's container with 'Storage Blob Data Contributor' role assignment. In case of user-assigned identity, make sure you add it under VM's identity and provide outputBlobUri / errorBlobUri and corresponding clientId in outputBlobManagedIdentity / errorBlobManagedIdentity parameter(s). In case of system-assigned identity, do not use outputBlobManagedIdentity / errorBlobManagedIdentity parameter(s). For more info, refer https://aka.ms/RunCommandManagedLinux" + // disable output blob if the policy settings has disableOutputBlobs set to true. var outputBlobSASRef *storage.Blob var outputBlobAppendClient *appendblob.Client var outputBlobAppendCreateOrReplaceError error @@ -951,6 +966,7 @@ func runCmd(ctx *log.Context, dir string, scriptFilePath string, cfg *handlerset scenario = "public-scriptUri" } + // Filter the inline script type here. ctx.Log("event", "prepare command", "scriptFile", scriptFilePath) // We need to kill previous extension process if exists before starting a new one. diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index 2ce23df..c6aff28 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -41,7 +41,7 @@ const ( ExitCode_GetExtensionPolicySettingsFailed = -301 ExitCode_ExtensionPolicyInvalid = -302 ExitCode_CommandNotAllowedByPolicy = -303 - ExitCode_ScriptNotAllowedByPolicy = -304 + ExitCode_ScriptTypeNotAllowedByPolicy = -304 // Unknown errors (-400s): ) diff --git a/internal/handlersettings/types.go b/internal/handlersettings/types.go index 49447a6..201dcaf 100644 --- a/internal/handlersettings/types.go +++ b/internal/handlersettings/types.go @@ -1,6 +1,7 @@ package handlersettings import ( + "github.com/Azure/run-command-handler-linux/internal/types" "github.com/pkg/errors" ) @@ -23,6 +24,14 @@ func (s HandlerSettings) ScriptURI() string { return s.PublicSettings.Source.ScriptURI } +func (s HandlerSettings) CommandId() string { + return s.PublicSettings.Source.CommandId // Only applicable when the ScriptType is a CommandId. +} + +func (s HandlerSettings) ScriptType() types.ScriptType { + return s.PublicSettings.Source.ScriptType +} + func (s HandlerSettings) ScriptSAS() string { return s.ProtectedSettings.SourceSASToken } @@ -69,7 +78,7 @@ func (s HandlerSettings) ReadArtifacts() ([]UnifiedArtifact, error) { func (s HandlerSettings) validate() error { // If installAsService is false, then the source has to be specified if !s.PublicSettings.InstallAsService { - if s.PublicSettings.Source == nil || (s.PublicSettings.Source.Script == "") == (s.PublicSettings.Source.ScriptURI == "") { + if s.PublicSettings.Source == nil || (s.PublicSettings.Source.Script == "") == (s.PublicSettings.Source.ScriptURI == "") { // Lourdes: check here also for scriptType? return errSourceNotSpecified } } @@ -149,8 +158,10 @@ type RunCommandManagedIdentity struct { } type ScriptSource struct { - Script string `json:"script"` - ScriptURI string `json:"scriptUri"` + Script string `json:"script"` + ScriptURI string `json:"scriptUri"` + CommandId string `json:"commandId"` + ScriptType types.ScriptType `json:"scriptType"` } type ParameterDefinition struct { diff --git a/internal/types/extensionpolicysettings.go b/internal/types/extensionpolicysettings.go index 7d24d83..14ef033 100644 --- a/internal/types/extensionpolicysettings.go +++ b/internal/types/extensionpolicysettings.go @@ -1,5 +1,10 @@ package types +import ( + "fmt" + "strings" +) + // ScriptType refers to the type of script being executed in a run command. // This type defintion matches the ScriptType definition in CRP in the Run Command Handler, // and should always be kept in sync with the ScriptType definition in RCv2 Windows. @@ -28,10 +33,11 @@ const ( // AllowedScriptType is a bitmask enum that defines which types of scripts run command is // allowed to execute based on customer policy. This should always match the AllowedScriptType in RCv2 Windows. -type AllowedScriptType int +type AllowedScriptTypeFlag uint32 const ( - AllowedCommandId int = 1 << iota + AllowedScriptNone AllowedScriptTypeFlag = 0 + AllowedCommandId AllowedScriptTypeFlag = 1 << iota Gallery Diagnostic Inline @@ -39,6 +45,36 @@ const ( AllowAll = AllowedCommandId | Gallery | Diagnostic | Inline | AllowedDownloaded ) +func StringToAllowedScriptTypeFlag(s string) (AllowedScriptTypeFlag, error) { + // lowercase the input to make the parsing case-insensitive + s = strings.ToLower(s) + // trim whitespace and split by comma + s = strings.TrimSpace(s) + parts := strings.Split(s, ",") + + var flag AllowedScriptTypeFlag + for _, part := range parts { + switch part { + case "inline": + flag |= Inline + case "alloweddownloaded": + flag |= AllowedDownloaded + case "gallery": + flag |= Gallery + case "diagnostic": + flag |= Diagnostic + case "allowedcommandid": + flag |= AllowedCommandId + case "allowall": + flag |= AllowAll + // TO-DO: consider the case where 'none' scripts are allowed to run. + default: + return 0, fmt.Errorf("policy blocks invalid script type: %s", part) + } + } + return flag, nil +} + // RCv2ExtensionPolicySettings defines the structure of the policy file for RCv2. // RequireSigning: describes the types of files that require signature verification. // FileRootCert: the root certificate used for signature verification. Required if RequireSigning is not "none". @@ -49,15 +85,64 @@ const ( type RCv2ExtensionPolicySettings struct { // RequireSigning FileType `json:"requireSigning"` // FileRootCert string `json:"fileRootCert,omitempty"` - DownloadedScriptsAllowlist []string `json:"downloadedScriptsAllowlist,omitempty"` - CommandIdAllowlist []string `json:"commandIdAllowlist,omitempty"` - RunAsUser string `json:"runAsUser,omitempty"` - LimitScripts AllowedScriptType `json:"limitScripts,omitempty"` - DisableOutputBlobs bool `json:"disableOutputBlobs,omitempty"` + DownloadedScriptsAllowlist []string `json:"downloadedScriptsAllowlist,omitempty"` + CommandIdAllowlist []string `json:"commandIdAllowlist,omitempty"` + RunAsUser string `json:"runAsUser,omitempty"` + LimitScripts string `json:"limitScripts,omitempty"` + DisableOutputBlobs bool `json:"disableOutputBlobs,omitempty"` } // This function is called from within the LoadExtensionPolicySettings function in extensionpolicysettings.go // to validate the format of our policy. func (rceps RCv2ExtensionPolicySettings) ValidateFormat() error { + flag, err := StringToAllowedScriptTypeFlag(string(rceps.LimitScripts)) + // Requirements: + // 1. If RequireSigning is not "none", FileRootCert must be present and non-empty. + // 2. LimitScripts must be a valid AllowedScriptType value. so map/check the value to the AllowedScriptTypeFlag bitmask. + if rceps.LimitScripts != "" { + if err != nil { + return fmt.Errorf("at least one of the values in LimitScripts is not a valid script type: %v", rceps.LimitScripts) + } + } + // 3. If DownloadedScriptsAllowlist is not empty, limit scripts must allow "downloaded" scripts. + if len(rceps.DownloadedScriptsAllowlist) > 0 { + if (flag & AllowedDownloaded) == 0 { + return fmt.Errorf("LimitScripts must allow 'downloaded' scripts if DownloadedScriptsAllowlist is not empty") + } + } + // 4. If CommandIdAllowlist is not empty, limit scripts must allow "commandId" scripts. + if len(rceps.CommandIdAllowlist) > 0 { + if (flag & AllowedCommandId) == 0 { + return fmt.Errorf("LimitScripts must allow 'commandId' scripts if CommandIdAllowlist is not empty") + } + } + return nil +} + +func CompareScriptTypeToAllowedScriptType(scriptType ScriptType, allowedScriptTypes AllowedScriptTypeFlag) error { + switch scriptType { + case InlineScript: + if (allowedScriptTypes & Inline) == 0 { + return fmt.Errorf("inline scripts are not allowed by policy") + } + case DownloadedScript: + if (allowedScriptTypes & AllowedDownloaded) == 0 { + return fmt.Errorf("downloaded scripts are not allowed by policy") + } + case GalleryScript: + if (allowedScriptTypes & Gallery) == 0 { + return fmt.Errorf("gallery scripts are not allowed by policy") + } + case DiagnosticScript: + if (allowedScriptTypes & Diagnostic) == 0 { + return fmt.Errorf("diagnostic scripts are not allowed by policy") + } + case CommandIdScript: + if (allowedScriptTypes & AllowedCommandId) == 0 { + return fmt.Errorf("commandId scripts are not allowed by policy") + } + default: + return fmt.Errorf("unknown script type: %v", scriptType) + } return nil } From eddf8f60745e8c2cd46853745dfee299fc1c08ca Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Sat, 18 Apr 2026 00:29:54 +0000 Subject: [PATCH 03/18] refactored to add validation against policy for downloaded scripts, commandIds, script types, and runasuser --- internal/cmds/cmds.go | 49 +++++----- internal/cmds/cmds_test.go | 4 +- internal/constants/exitcodes.go | 4 +- .../extensionpolicysettingsrc.go | 97 +++++++++++++++++++ internal/types/extensionpolicysettings.go | 3 + 5 files changed, 127 insertions(+), 30 deletions(-) create mode 100644 internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 0637d7f..aea1fe1 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -17,9 +17,11 @@ import ( "strings" "time" + "github.com/Azure/azure-extension-platform/pkg/extensionerrors" "github.com/Azure/azure-extension-platform/pkg/extensionevents" "github.com/Azure/azure-extension-platform/pkg/extensionpolicysettings" "github.com/Azure/azure-extension-platform/pkg/handlerenv" + "github.com/Azure/azure-extension-platform/pkg/hashutils" "github.com/Azure/azure-extension-platform/pkg/logging" "github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -29,6 +31,7 @@ import ( "github.com/Azure/run-command-handler-linux/internal/commandProcessor" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/exec" + "github.com/Azure/run-command-handler-linux/internal/extensionpolicysettingsrc" "github.com/Azure/run-command-handler-linux/internal/files" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/Azure/run-command-handler-linux/internal/immediatecmds" @@ -218,22 +221,11 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma var rceps *types.RCv2ExtensionPolicySettings if _, err := os.Stat(policyPath); err == nil { - ExtensionPolicyManagerPtr, err = extensionpolicysettings.NewExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings](policyPath) + err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr, policyPath, rceps) if err != nil { - return "", "", errors.Wrap(err, "failed to create extension policy settings manager"), constants.ExitCode_LoadExtensionPolicySettingsFailed - } - - err = ExtensionPolicyManagerPtr.LoadExtensionPolicySettings() - if err != nil { - return "", "", errors.Wrap(err, "failed to load extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed - } else { - rceps, err = ExtensionPolicyManagerPtr.GetSettings() - - if err != nil { - return "", "", errors.Wrap(err, "failed to get extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed - } - ctx.Log("message", "successfully loaded extension policy settings", "settings", rceps) + return "", "", errors.Wrap(err, "failed to initialize extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed } + ctx.Log("message", "successfully initialized extension policy settings") } else if os.IsNotExist(err) { ctx.Log("message", "extension policy settings file does not exist. No policy applied.", "error", err) ExtensionPolicyManagerPtr = nil @@ -241,21 +233,18 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma return "", "", errors.Wrap(err, "failed to stat extension policy settings file"), constants.ExitCode_LoadExtensionPolicySettingsFailed } - // Limit scripts by type before downloading them. + // Validate handler settings against policy settings. if ExtensionPolicyManagerPtr != nil && rceps != nil { - allowedScriptType, err := types.StringToAllowedScriptTypeFlag(rceps.LimitScripts) - if err != nil { // We should not hit this because we already validte the policy settings earlier. - return "", "", errors.Wrap(err, "failed to parse allowed script types"), constants.ExitCode_ExtensionPolicyInvalid - } - // Compare the script type of the command with the allowed script types in the policy. - err = types.CompareScriptTypeToAllowedScriptType(cfg.ScriptType(), allowedScriptType) - if err != nil { - return "", "", errors.Wrap(err, "script type is not allowed by policy"), constants.ExitCode_ScriptTypeNotAllowedByPolicy + if err = extensionpolicysettingsrc.InitialValidateHandlerSettingsAgainstPolicy(&cfg, rceps); err != nil { + return "", "", err, constants.ExitCode_HandlerSettingsViolatePolicy } } dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) - scriptFilePath, err := downloadScript(ctx, dir, &cfg) + scriptFilePath, err := downloadScript(ctx, dir, &cfg, rceps) + if err != nil && errors.Is(err, extensionerrors.ErrItemNotInAllowlist) { + return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist"), constants.ExitCode_DownloadedScriptBlockedByPolicy + } if err != nil { errMessage := fmt.Sprintf("Failed to download script: %v due to: %v", download.GetUriForLogging(cfg.ScriptURI()), err) extensionEvents.LogErrorEvent("enable", errMessage) @@ -276,7 +265,7 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma blobCreateOrReplaceError := "Error creating AppendBlob '%s' using SAS token or Managed identity. Please use a valid blob SAS URI with [read, append, create, write] permissions OR managed identity. If managed identity is used, make sure Azure blob and identity exist, and identity has been given access to storage blob's container with 'Storage Blob Data Contributor' role assignment. In case of user-assigned identity, make sure you add it under VM's identity and provide outputBlobUri / errorBlobUri and corresponding clientId in outputBlobManagedIdentity / errorBlobManagedIdentity parameter(s). In case of system-assigned identity, do not use outputBlobManagedIdentity / errorBlobManagedIdentity parameter(s). For more info, refer https://aka.ms/RunCommandManagedLinux" - // disable output blob if the policy settings has disableOutputBlobs set to true. + // TO-DO: disable output blob if the policy settings has disableOutputBlobs set to true. var outputBlobSASRef *storage.Blob var outputBlobAppendClient *appendblob.Client var outputBlobAppendCreateOrReplaceError error @@ -892,7 +881,7 @@ func createDummyStatusFilesIfNeeded(ctx log.Logger, mrseqFilesNameList *list.Lis // downloadScript downloads the script file specified in cfg into dir (creates if does // not exist) and takes storage credentials specified in cfg into account. -func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings) (string, error) { +func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings, rceps *types.RCv2ExtensionPolicySettings) (string, error) { // - prepare the output directory for files and the command output // - create the directory if missing ctx.Log("event", "creating output directory", "path", dir) @@ -917,6 +906,14 @@ func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSe } scriptFilePath = file ctx.Log("event", "download complete", "output", dir) + + if rceps != nil { + // Assume the downloaded script type is already allowed, since this was already validated earlier in enable(). + err = extensionpolicysettings.ValidateFileHashInAllowlist(scriptFilePath, rceps.DownloadedScriptsAllowlist, hashutils.HashTypeSHA256) + if err != nil { + return scriptFilePath, errors.Wrapf(err, "file %s blocked by policy", scriptFilePath) + } + } } return scriptFilePath, nil } diff --git a/internal/cmds/cmds_test.go b/internal/cmds/cmds_test.go index 72f4e76..922876b 100755 --- a/internal/cmds/cmds_test.go +++ b/internal/cmds/cmds_test.go @@ -551,7 +551,7 @@ func Test_downloadScriptUri(t *testing.T) { PublicSettings: handlersettings.PublicSettings{ Source: &handlersettings.ScriptSource{ScriptURI: srv.URL + "/bytes/10"}, }, - }) + }, nil) require.Nil(t, err) // check the downloaded file @@ -744,7 +744,7 @@ func Test_downloadScriptUri_BySASFailsSucceedsByManagedIdentity(t *testing.T) { ClientId: "00b64c6a-6dbf-41e0-8707-74132d5cf53f", }, }, - }) + }, nil) require.Nil(t, err) files.UseMockSASDownloadFailure = false } diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index c6aff28..9181572 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -40,8 +40,8 @@ const ( ExitCode_LoadExtensionPolicySettingsFailed = -300 ExitCode_GetExtensionPolicySettingsFailed = -301 ExitCode_ExtensionPolicyInvalid = -302 - ExitCode_CommandNotAllowedByPolicy = -303 - ExitCode_ScriptTypeNotAllowedByPolicy = -304 + ExitCode_HandlerSettingsViolatePolicy = -303 + ExitCode_DownloadedScriptBlockedByPolicy = -304 // Unknown errors (-400s): ) diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go new file mode 100644 index 0000000..05849f9 --- /dev/null +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go @@ -0,0 +1,97 @@ +package extensionpolicysettingsrc + +import ( + "fmt" + "strings" + + "github.com/Azure/azure-extension-platform/pkg/extensionpolicysettings" + "github.com/Azure/run-command-handler-linux/internal/handlersettings" + "github.com/Azure/run-command-handler-linux/internal/types" + "github.com/pkg/errors" +) + +func InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings], + policyPath string, + rceps *types.RCv2ExtensionPolicySettings) error { + ExtensionPolicyManagerPtr, err := extensionpolicysettings.NewExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings](policyPath) + if err != nil { + return errors.Wrap(err, "failed to create extension policy settings manager") + } + + err = ExtensionPolicyManagerPtr.LoadExtensionPolicySettings() + if err != nil { + return errors.Wrap(err, "failed to load extension policy settings") + } else { + rceps, err = ExtensionPolicyManagerPtr.GetSettings() + + if err != nil { + return errors.Wrap(err, "failed to get extension policy settings") + } + } + return nil +} + +func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) error { + if policy == nil { + return fmt.Errorf("no policy provided") + } + if err := ValidateScriptTypeAgainstPolicy(settings.ScriptType(), policy.LimitScripts); err != nil { + return err + } + if settings.ScriptType() == types.CommandIdScript { + if err := ValidateCommandId(settings, policy); err != nil { + return err + } + } + if policy.RunAsUser != "" { + if err := ValidateRunAsUser(settings, policy); err != nil { + return err + } + } + if policy.DisableOutputBlobs { + ValidateOutputBlob(settings, policy) + } + return nil +} + +func ValidateScriptTypeAgainstPolicy(scriptType types.ScriptType, allowedScriptTypesString string) error { + allowedScriptTypes, _ := types.StringToAllowedScriptTypeFlag(allowedScriptTypesString) + // Compare the script type of the command with the allowed script types in the policy. + err := types.CompareScriptTypeToAllowedScriptType(scriptType, allowedScriptTypes) + if err != nil { + return errors.Wrapf(err, "script type %s is not allowed by policy", scriptType) + } + return nil +} + +func ValidateCommandId(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) error { + settingsCommandId := settings.CommandId() + allowedCommandIds := policy.CommandIdAllowlist + + if len(allowedCommandIds) == 0 { + // if list is empty, all commandIds are allowed + return nil + } + return extensionpolicysettings.ValidateValueInAllowlist(settingsCommandId, allowedCommandIds) +} + +func ValidateRunAsUser(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) error { + settingsRunAsUser := strings.ToLower(strings.TrimSpace(settings.RunAsUser)) + policyRunAsUser := strings.ToLower(strings.TrimSpace(policy.RunAsUser)) + + if strings.Compare(settingsRunAsUser, policyRunAsUser) != 0 { + return fmt.Errorf("RunAsUser '%s' in settings does not match RunAsUser '%s' in policy", settingsRunAsUser, policyRunAsUser) + } + return nil +} + +func ValidateOutputBlob(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) { + if policy.DisableOutputBlobs { + // Log a warning that output blobs are disabled by policy. The command will still execute, but no output blobs will be created. + if settings.OutputBlobURI != "" { + fmt.Println("Warning: Output blobs are disabled by policy. The provided output blob URI will be ignored and no output blobs will be created for this command.") + } else { + fmt.Println("Warning: Output blobs are disabled by policy. No output blobs will be created for this command.") + } + } +} diff --git a/internal/types/extensionpolicysettings.go b/internal/types/extensionpolicysettings.go index 14ef033..82addd4 100644 --- a/internal/types/extensionpolicysettings.go +++ b/internal/types/extensionpolicysettings.go @@ -119,6 +119,9 @@ func (rceps RCv2ExtensionPolicySettings) ValidateFormat() error { return nil } +// This function compares a script type (of type ScriptType, defined in this file) to the allowed script types +// (of type AllowedScriptTypeFlag, also defined in this file) listed in the policy. These values and mappings +// are specific to Run Command, hence why they are defined here and not in the shared library. func CompareScriptTypeToAllowedScriptType(scriptType ScriptType, allowedScriptTypes AllowedScriptTypeFlag) error { switch scriptType { case InlineScript: From 26aeadcdd7106d0a394f198a4694e7a3ef1082da Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Mon, 20 Apr 2026 18:52:04 +0000 Subject: [PATCH 04/18] refactored policy files + added UTs (wip) --- internal/cmds/cmds.go | 6 +- .../extensionpolicysettingsrc.go | 23 +- .../extensionpolicysettingsrc_test.go | 252 ++++++++++++++++ .../types.go} | 42 +-- .../extensionpolicysettingsrc/types_test.go | 278 ++++++++++++++++++ internal/handlersettings/types.go | 30 +- 6 files changed, 583 insertions(+), 48 deletions(-) create mode 100644 internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go rename internal/{types/extensionpolicysettings.go => extensionpolicysettingsrc/types.go} (78%) create mode 100644 internal/extensionpolicysettingsrc/types_test.go diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index aea1fe1..8af04ed 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -216,9 +216,9 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma // Load extension policy settings. // If policy file exists, load the policy. If not, then don't load. - var ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings] + var ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[extensionpolicysettingsrc.RCv2ExtensionPolicySettings] policyPath := filepath.Join(h.HandlerEnvironment.ConfigFolder, constants.PolicyFileName) - var rceps *types.RCv2ExtensionPolicySettings + var rceps *extensionpolicysettingsrc.RCv2ExtensionPolicySettings if _, err := os.Stat(policyPath); err == nil { err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr, policyPath, rceps) @@ -881,7 +881,7 @@ func createDummyStatusFilesIfNeeded(ctx log.Logger, mrseqFilesNameList *list.Lis // downloadScript downloads the script file specified in cfg into dir (creates if does // not exist) and takes storage credentials specified in cfg into account. -func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings, rceps *types.RCv2ExtensionPolicySettings) (string, error) { +func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings, rceps *extensionpolicysettingsrc.RCv2ExtensionPolicySettings) (string, error) { // - prepare the output directory for files and the command output // - create the directory if missing ctx.Log("event", "creating output directory", "path", dir) diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go index 05849f9..7a4bf3d 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go @@ -6,14 +6,13 @@ import ( "github.com/Azure/azure-extension-platform/pkg/extensionpolicysettings" "github.com/Azure/run-command-handler-linux/internal/handlersettings" - "github.com/Azure/run-command-handler-linux/internal/types" "github.com/pkg/errors" ) -func InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings], +func InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], policyPath string, - rceps *types.RCv2ExtensionPolicySettings) error { - ExtensionPolicyManagerPtr, err := extensionpolicysettings.NewExtensionPolicySettingsManager[types.RCv2ExtensionPolicySettings](policyPath) + rceps *RCv2ExtensionPolicySettings) error { + ExtensionPolicyManagerPtr, err := extensionpolicysettings.NewExtensionPolicySettingsManager[RCv2ExtensionPolicySettings](policyPath) if err != nil { return errors.Wrap(err, "failed to create extension policy settings manager") } @@ -31,14 +30,14 @@ func InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr *extensionpolic return nil } -func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) error { +func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { if policy == nil { return fmt.Errorf("no policy provided") } if err := ValidateScriptTypeAgainstPolicy(settings.ScriptType(), policy.LimitScripts); err != nil { return err } - if settings.ScriptType() == types.CommandIdScript { + if settings.ScriptType() == handlersettings.CommandIdScript { if err := ValidateCommandId(settings, policy); err != nil { return err } @@ -54,17 +53,17 @@ func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.Handl return nil } -func ValidateScriptTypeAgainstPolicy(scriptType types.ScriptType, allowedScriptTypesString string) error { - allowedScriptTypes, _ := types.StringToAllowedScriptTypeFlag(allowedScriptTypesString) +func ValidateScriptTypeAgainstPolicy(scriptType handlersettings.ScriptType, allowedScriptTypesString string) error { + allowedScriptTypes, _ := StringToAllowedScriptTypeFlag(allowedScriptTypesString) // Compare the script type of the command with the allowed script types in the policy. - err := types.CompareScriptTypeToAllowedScriptType(scriptType, allowedScriptTypes) + err := CompareScriptTypeToAllowedScriptType(scriptType, allowedScriptTypes) if err != nil { return errors.Wrapf(err, "script type %s is not allowed by policy", scriptType) } return nil } -func ValidateCommandId(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) error { +func ValidateCommandId(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { settingsCommandId := settings.CommandId() allowedCommandIds := policy.CommandIdAllowlist @@ -75,7 +74,7 @@ func ValidateCommandId(settings *handlersettings.HandlerSettings, policy *types. return extensionpolicysettings.ValidateValueInAllowlist(settingsCommandId, allowedCommandIds) } -func ValidateRunAsUser(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) error { +func ValidateRunAsUser(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { settingsRunAsUser := strings.ToLower(strings.TrimSpace(settings.RunAsUser)) policyRunAsUser := strings.ToLower(strings.TrimSpace(policy.RunAsUser)) @@ -85,7 +84,7 @@ func ValidateRunAsUser(settings *handlersettings.HandlerSettings, policy *types. return nil } -func ValidateOutputBlob(settings *handlersettings.HandlerSettings, policy *types.RCv2ExtensionPolicySettings) { +func ValidateOutputBlob(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) { if policy.DisableOutputBlobs { // Log a warning that output blobs are disabled by policy. The command will still execute, but no output blobs will be created. if settings.OutputBlobURI != "" { diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go new file mode 100644 index 0000000..0c50d68 --- /dev/null +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go @@ -0,0 +1,252 @@ +package extensionpolicysettingsrc + +import ( + "io" + "os" + "path/filepath" + "testing" + + "github.com/Azure/azure-extension-platform/pkg/extensionpolicysettings" + "github.com/Azure/run-command-handler-linux/internal/handlersettings" + "github.com/stretchr/testify/require" +) + +func makeSettings(scriptType handlersettings.ScriptType, commandID string, runAsUser string, outputBlobURI string) *handlersettings.HandlerSettings { + return &handlersettings.HandlerSettings{ + PublicSettings: handlersettings.PublicSettings{ + Source: &handlersettings.ScriptSource{ + ScriptType: scriptType, + CommandId: commandID, + }, + RunAsUser: runAsUser, + OutputBlobURI: outputBlobURI, + }, + } +} + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + old := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + + os.Stdout = w + fn() + _ = w.Close() + os.Stdout = old + + out, err := io.ReadAll(r) + require.NoError(t, err) + _ = r.Close() + + return string(out) +} + +func TestInitializeExtensionPolicySettings_InvalidPath_ReturnsError(t *testing.T) { + var mgr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] + out := &RCv2ExtensionPolicySettings{} + + err := InitializeExtensionPolicySettings(mgr, "/definitely/not/found/policy.json", out) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to") +} + +func TestInitializeExtensionPolicySettings_ValidFile_ReturnsNil(t *testing.T) { + tmpDir := t.TempDir() + policyPath := filepath.Join(tmpDir, "policy.json") + + // Minimal valid payload for current ValidateFormat behavior. + err := os.WriteFile(policyPath, []byte("{}"), 0600) + require.NoError(t, err) + + var mgr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] + out := &RCv2ExtensionPolicySettings{} + + err = InitializeExtensionPolicySettings(mgr, policyPath, out) + require.NoError(t, err) +} + +func TestInitializeExtensionPolicySettings_CurrentBehavior_DoesNotPopulateOutputStruct(t *testing.T) { + tmpDir := t.TempDir() + policyPath := filepath.Join(tmpDir, "policy.json") + + payload := `{"limitScripts":"inline","runAsUser":"alice"}` + err := os.WriteFile(policyPath, []byte(payload), 0600) + require.NoError(t, err) + + var mgr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] + out := &RCv2ExtensionPolicySettings{} + + err = InitializeExtensionPolicySettings(mgr, policyPath, out) + require.NoError(t, err) + + // Documents current implementation behavior (pointer reassignment inside function). + require.Equal(t, "", out.LimitScripts) + require.Equal(t, "", out.RunAsUser) +} + +func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { + t.Run("nil policy", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", "", "") + err := InitialValidateHandlerSettingsAgainstPolicy(settings, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "no policy provided") + }) + + t.Run("script type blocked by policy", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", "", "") + policy := &RCv2ExtensionPolicySettings{ + LimitScripts: "gallery", + } + + err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + require.Error(t, err) + require.Contains(t, err.Error(), "script type inline is not allowed by policy") + }) + + t.Run("command id not in allowlist", func(t *testing.T) { + settings := makeSettings(handlersettings.CommandIdScript, "restartVM", "", "") + policy := &RCv2ExtensionPolicySettings{ + LimitScripts: "allowedcommandid", + CommandIdAllowlist: []string{"safeCommand"}, + } + + err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + require.Error(t, err) + }) + + t.Run("runAs mismatch", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", "bob", "") + policy := &RCv2ExtensionPolicySettings{ + LimitScripts: "inline", + RunAsUser: "alice", + } + + err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + require.Error(t, err) + require.Contains(t, err.Error(), "does not match") + }) + + t.Run("all checks pass", func(t *testing.T) { + settings := makeSettings(handlersettings.CommandIdScript, "safeCommand", " Alice ", "https://example/blob") + policy := &RCv2ExtensionPolicySettings{ + LimitScripts: "allowall", + CommandIdAllowlist: []string{"safeCommand"}, + RunAsUser: "alice", + DisableOutputBlobs: true, + } + + err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + require.NoError(t, err) + }) +} + +func TestValidateScriptTypeAgainstPolicy(t *testing.T) { + t.Run("allowed", func(t *testing.T) { + err := ValidateScriptTypeAgainstPolicy(handlersettings.InlineScript, "inline") + require.NoError(t, err) + }) + + t.Run("blocked", func(t *testing.T) { + err := ValidateScriptTypeAgainstPolicy(handlersettings.GalleryScript, "inline") + require.Error(t, err) + require.Contains(t, err.Error(), "script type gallery is not allowed by policy") + }) + + t.Run("invalid policy token currently treated as blocked", func(t *testing.T) { + err := ValidateScriptTypeAgainstPolicy(handlersettings.InlineScript, "notARealScriptType") + require.Error(t, err) + require.Contains(t, err.Error(), "script type inline is not allowed by policy") + }) +} + +func TestValidateCommandId(t *testing.T) { + t.Run("empty allowlist allows all", func(t *testing.T) { + settings := makeSettings(handlersettings.CommandIdScript, "anything", "", "") + policy := &RCv2ExtensionPolicySettings{ + CommandIdAllowlist: nil, + } + err := ValidateCommandId(settings, policy) + require.NoError(t, err) + }) + + t.Run("value present in allowlist", func(t *testing.T) { + settings := makeSettings(handlersettings.CommandIdScript, "safeCommand", "", "") + policy := &RCv2ExtensionPolicySettings{ + CommandIdAllowlist: []string{"safeCommand", "other"}, + } + err := ValidateCommandId(settings, policy) + require.NoError(t, err) + }) + + t.Run("value missing from allowlist", func(t *testing.T) { + settings := makeSettings(handlersettings.CommandIdScript, "restartVM", "", "") + policy := &RCv2ExtensionPolicySettings{ + CommandIdAllowlist: []string{"safeCommand", "other"}, + } + err := ValidateCommandId(settings, policy) + require.Error(t, err) + }) +} + +func TestValidateRunAsUser(t *testing.T) { + t.Run("match with whitespace and case differences", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", " Alice ", "") + policy := &RCv2ExtensionPolicySettings{ + RunAsUser: "alice", + } + err := ValidateRunAsUser(settings, policy) + require.NoError(t, err) + }) + + t.Run("mismatch", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", "bob", "") + policy := &RCv2ExtensionPolicySettings{ + RunAsUser: "alice", + } + err := ValidateRunAsUser(settings, policy) + require.Error(t, err) + require.Contains(t, err.Error(), "does not match") + }) +} + +func TestValidateOutputBlob(t *testing.T) { + t.Run("policy does not disable output blobs prints nothing", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", "", "https://example/blob") + policy := &RCv2ExtensionPolicySettings{ + DisableOutputBlobs: false, + } + + out := captureStdout(t, func() { + ValidateOutputBlob(settings, policy) + }) + require.Equal(t, "", out) + }) + + t.Run("disabled with output blob uri prints ignore warning", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", "", "https://example/blob") + policy := &RCv2ExtensionPolicySettings{ + DisableOutputBlobs: true, + } + + out := captureStdout(t, func() { + ValidateOutputBlob(settings, policy) + }) + require.Contains(t, out, "Output blobs are disabled by policy") + require.Contains(t, out, "provided output blob URI will be ignored") + }) + + t.Run("disabled without output blob uri prints no blob warning", func(t *testing.T) { + settings := makeSettings(handlersettings.InlineScript, "", "", "") + policy := &RCv2ExtensionPolicySettings{ + DisableOutputBlobs: true, + } + + out := captureStdout(t, func() { + ValidateOutputBlob(settings, policy) + }) + require.Contains(t, out, "Output blobs are disabled by policy") + require.Contains(t, out, "No output blobs will be created") + }) +} diff --git a/internal/types/extensionpolicysettings.go b/internal/extensionpolicysettingsrc/types.go similarity index 78% rename from internal/types/extensionpolicysettings.go rename to internal/extensionpolicysettingsrc/types.go index 82addd4..64c73a9 100644 --- a/internal/types/extensionpolicysettings.go +++ b/internal/extensionpolicysettingsrc/types.go @@ -1,23 +1,10 @@ -package types +package extensionpolicysettingsrc import ( "fmt" "strings" -) - -// ScriptType refers to the type of script being executed in a run command. -// This type defintion matches the ScriptType definition in CRP in the Run Command Handler, -// and should always be kept in sync with the ScriptType definition in RCv2 Windows. -// None is defined in the case where no script is passed down, which is a valid scenario. -type ScriptType string -const ( - InlineScript ScriptType = "inline" - DownloadedScript ScriptType = "downloaded" - GalleryScript ScriptType = "gallery" - DiagnosticScript ScriptType = "diagnostic" - CommandIdScript ScriptType = "commandId" - NoneScript ScriptType = "none" + "github.com/Azure/run-command-handler-linux/internal/handlersettings" ) // This refers *specifically* to file types that require signature verification @@ -36,13 +23,13 @@ const ( type AllowedScriptTypeFlag uint32 const ( - AllowedScriptNone AllowedScriptTypeFlag = 0 - AllowedCommandId AllowedScriptTypeFlag = 1 << iota + AllowedCommandId = 1 << iota Gallery Diagnostic Inline AllowedDownloaded - AllowAll = AllowedCommandId | Gallery | Diagnostic | Inline | AllowedDownloaded + AllowAll = AllowedCommandId | Gallery | Diagnostic | Inline | AllowedDownloaded + AllowedScriptNone = 0 ) func StringToAllowedScriptTypeFlag(s string) (AllowedScriptTypeFlag, error) { @@ -54,6 +41,7 @@ func StringToAllowedScriptTypeFlag(s string) (AllowedScriptTypeFlag, error) { var flag AllowedScriptTypeFlag for _, part := range parts { + part = strings.TrimSpace(part) switch part { case "inline": flag |= Inline @@ -69,7 +57,7 @@ func StringToAllowedScriptTypeFlag(s string) (AllowedScriptTypeFlag, error) { flag |= AllowAll // TO-DO: consider the case where 'none' scripts are allowed to run. default: - return 0, fmt.Errorf("policy blocks invalid script type: %s", part) + return 0, fmt.Errorf("Unknown script type in policy: %s", part) } } return flag, nil @@ -107,13 +95,13 @@ func (rceps RCv2ExtensionPolicySettings) ValidateFormat() error { // 3. If DownloadedScriptsAllowlist is not empty, limit scripts must allow "downloaded" scripts. if len(rceps.DownloadedScriptsAllowlist) > 0 { if (flag & AllowedDownloaded) == 0 { - return fmt.Errorf("LimitScripts must allow 'downloaded' scripts if DownloadedScriptsAllowlist is not empty") + return fmt.Errorf("DownloadedScriptsAllowlist not empty, but LimitScripts does not allow 'downloaded' scripts") } } // 4. If CommandIdAllowlist is not empty, limit scripts must allow "commandId" scripts. if len(rceps.CommandIdAllowlist) > 0 { if (flag & AllowedCommandId) == 0 { - return fmt.Errorf("LimitScripts must allow 'commandId' scripts if CommandIdAllowlist is not empty") + return fmt.Errorf("CommandIdAllowlist not empty, but LimitScripts does not allow 'commandId' scripts") } } return nil @@ -122,25 +110,25 @@ func (rceps RCv2ExtensionPolicySettings) ValidateFormat() error { // This function compares a script type (of type ScriptType, defined in this file) to the allowed script types // (of type AllowedScriptTypeFlag, also defined in this file) listed in the policy. These values and mappings // are specific to Run Command, hence why they are defined here and not in the shared library. -func CompareScriptTypeToAllowedScriptType(scriptType ScriptType, allowedScriptTypes AllowedScriptTypeFlag) error { +func CompareScriptTypeToAllowedScriptType(scriptType handlersettings.ScriptType, allowedScriptTypes AllowedScriptTypeFlag) error { switch scriptType { - case InlineScript: + case handlersettings.InlineScript: if (allowedScriptTypes & Inline) == 0 { return fmt.Errorf("inline scripts are not allowed by policy") } - case DownloadedScript: + case handlersettings.DownloadedScript: if (allowedScriptTypes & AllowedDownloaded) == 0 { return fmt.Errorf("downloaded scripts are not allowed by policy") } - case GalleryScript: + case handlersettings.GalleryScript: if (allowedScriptTypes & Gallery) == 0 { return fmt.Errorf("gallery scripts are not allowed by policy") } - case DiagnosticScript: + case handlersettings.DiagnosticScript: if (allowedScriptTypes & Diagnostic) == 0 { return fmt.Errorf("diagnostic scripts are not allowed by policy") } - case CommandIdScript: + case handlersettings.CommandIdScript: if (allowedScriptTypes & AllowedCommandId) == 0 { return fmt.Errorf("commandId scripts are not allowed by policy") } diff --git a/internal/extensionpolicysettingsrc/types_test.go b/internal/extensionpolicysettingsrc/types_test.go new file mode 100644 index 0000000..efb4515 --- /dev/null +++ b/internal/extensionpolicysettingsrc/types_test.go @@ -0,0 +1,278 @@ +package extensionpolicysettingsrc + +import ( + "testing" + + "github.com/Azure/run-command-handler-linux/internal/handlersettings" +) + +func TestTypeDefinitions_AreStable(t *testing.T) { + t.Run("file type values are stable", func(t *testing.T) { + if got := string(All); got != "all" { + t.Fatalf("All = %q, want %q", got, "all") + } + if got := string(NoFiles); got != "none" { + t.Fatalf("NoFiles = %q, want %q", got, "none") + } + if got := string(Scripts); got != "scripts" { + t.Fatalf("Scripts = %q, want %q", got, "scripts") + } + }) + + t.Run("allowed script flag values are stable", func(t *testing.T) { + tests := []struct { + name string + got AllowedScriptTypeFlag + want AllowedScriptTypeFlag + }{ + {name: "AllowedScriptNone", got: AllowedScriptNone, want: 0}, + {name: "AllowedCommandId", got: AllowedCommandId, want: 1}, + {name: "Gallery", got: Gallery, want: 2}, + {name: "Diagnostic", got: Diagnostic, want: 4}, + {name: "Inline", got: Inline, want: 8}, + {name: "AllowedDownloaded", got: AllowedDownloaded, want: 16}, + {name: "AllowAll", got: AllowAll, want: 31}, + } + + for _, tt := range tests { + if tt.got != tt.want { + t.Fatalf("%s = %d, want %d", tt.name, tt.got, tt.want) + } + } + }) + + t.Run("script type values are stable", func(t *testing.T) { + tests := []struct { + name string + got handlersettings.ScriptType + want handlersettings.ScriptType + }{ + {name: "InlineScript", got: handlersettings.InlineScript, want: "inline"}, + {name: "DownloadedScript", got: handlersettings.DownloadedScript, want: "downloaded"}, + {name: "GalleryScript", got: handlersettings.GalleryScript, want: "gallery"}, + {name: "DiagnosticScript", got: handlersettings.DiagnosticScript, want: "diagnostic"}, + {name: "CommandIdScript", got: handlersettings.CommandIdScript, want: "commandId"}, + {name: "NoneScript", got: handlersettings.NoneScript, want: "none"}, + } + + for _, tt := range tests { + if tt.got != tt.want { + t.Fatalf("%s = %q, want %q", tt.name, tt.got, tt.want) + } + } + }) +} + +func TestStringToAllowedScriptTypeFlag(t *testing.T) { + tests := []struct { + name string + input string + want AllowedScriptTypeFlag + wantErr string + }{ + { + name: "inline", + input: "inline", + want: Inline, + }, + { + name: "inline plus gallery", + input: "inline,gallery", + want: Inline | Gallery, + }, + { + name: "allowed command id plus gallery plus inline", + input: "allowedcommandid,gallery,inline", + want: AllowedCommandId | Gallery | Inline, + }, + { + name: "all explicit types", + input: "alloweddownloaded,allowedcommandid,diagnostic,inline,gallery", + want: AllowedDownloaded | AllowedCommandId | Diagnostic | Inline | Gallery, + }, + { + name: "allow all", + input: "allowall", + want: AllowAll, + }, + { + name: "whitespace and capitalization", + input: " InLiNe , GALLERY , allowedCommandId ", + want: Inline | Gallery | AllowedCommandId, + }, + { + name: "unknown string", + input: "inline,banana", + wantErr: "Unknown script type in policy: banana", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := StringToAllowedScriptTypeFlag(tt.input) + + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error %q, got nil", tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Fatalf("error = %q, want %q", err.Error(), tt.wantErr) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Fatalf("got %d, want %d", got, tt.want) + } + }) + } +} + +func TestValidateFormat(t *testing.T) { + tests := []struct { + name string + input RCv2ExtensionPolicySettings + wantErr string + }{ + { + name: "valid policy", + input: RCv2ExtensionPolicySettings{ + LimitScripts: "alloweddownloaded,allowedcommandid,diagnostic,inline,gallery", + DownloadedScriptsAllowlist: []string{"hash1"}, + CommandIdAllowlist: []string{"cmd1"}, + RunAsUser: "alice", + DisableOutputBlobs: true, + }, + }, + { + name: "invalid limit scripts value", + input: RCv2ExtensionPolicySettings{ + LimitScripts: "inline,notARealType", + }, + wantErr: "at least one of the values in LimitScripts is not a valid script type: inline,notARealType", + }, + { + name: "downloaded allowlist present but downloaded blocked", + input: RCv2ExtensionPolicySettings{ + LimitScripts: "inline,gallery", + DownloadedScriptsAllowlist: []string{"hash1"}, + }, + wantErr: "DownloadedScriptsAllowlist not empty, but LimitScripts does not allow 'downloaded' scripts", + }, + { + name: "command id allowlist present but command id blocked", + input: RCv2ExtensionPolicySettings{ + LimitScripts: "inline,gallery", + CommandIdAllowlist: []string{"cmd1"}, + }, + wantErr: "CommandIdAllowlist not empty, but LimitScripts does not allow 'commandId' scripts", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.input.ValidateFormat() + + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error %q, got nil", tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Fatalf("error = %q, want %q", err.Error(), tt.wantErr) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + +func TestCompareScriptTypeToAllowedScriptType(t *testing.T) { + tests := []struct { + name string + scriptType handlersettings.ScriptType + allowed AllowedScriptTypeFlag + wantErr string + }{ + // { + // name: "none allowed gallery denied", + // scriptType: handlersettings.GalleryScript, + // allowed: AllowedScriptNone, + // wantErr: "gallery scripts are not allowed by policy", + // }, + { + name: "allow all inline", + scriptType: handlersettings.InlineScript, + allowed: AllowAll, + }, + { + name: "allow all downloaded", + scriptType: handlersettings.DownloadedScript, + allowed: AllowAll, + }, + { + name: "allow all gallery", + scriptType: handlersettings.GalleryScript, + allowed: AllowAll, + }, + { + name: "allow all diagnostic", + scriptType: handlersettings.DiagnosticScript, + allowed: AllowAll, + }, + { + name: "allow all command id", + scriptType: handlersettings.CommandIdScript, + allowed: AllowAll, + }, + { + name: "diagnostic only inline denied", + scriptType: handlersettings.InlineScript, + allowed: Diagnostic, + wantErr: "inline scripts are not allowed by policy", + }, + { + name: "allowed downloaded permits downloaded", + scriptType: handlersettings.DownloadedScript, + allowed: AllowedDownloaded, + }, + { + name: "unknown script type", + scriptType: handlersettings.ScriptType("made-up"), + allowed: AllowAll, + wantErr: "unknown script type: made-up", + }, + { + name: "none script currently treated as unknown", + scriptType: handlersettings.NoneScript, + allowed: AllowAll, + wantErr: "unknown script type: none", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := CompareScriptTypeToAllowedScriptType(tt.scriptType, tt.allowed) + + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error %q, got nil", tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Fatalf("error = %q, want %q", err.Error(), tt.wantErr) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/internal/handlersettings/types.go b/internal/handlersettings/types.go index 201dcaf..175aa66 100644 --- a/internal/handlersettings/types.go +++ b/internal/handlersettings/types.go @@ -1,7 +1,6 @@ package handlersettings import ( - "github.com/Azure/run-command-handler-linux/internal/types" "github.com/pkg/errors" ) @@ -11,6 +10,25 @@ type HandlerSettings struct { ProtectedSettings } +// ScriptType refers to the type of script being executed in a run command. +// This type defintion matches the ScriptType definition in CRP in the Run Command Handler, +// and should always be kept in sync with the ScriptType definition in RCv2 Windows. +// None is defined in the case where no script is passed down, which is a valid scenario. +// +// Note: although this is a property that was introduced due to Extension Policy Settings, +// it is defined here to avoid a circular dependency between handlersettings and extensionpolicysettingsrc. +// CRP has been modified to also pass down the ScriptType, so it is appropriately defined here. +type ScriptType string + +const ( + InlineScript ScriptType = "inline" + DownloadedScript ScriptType = "downloaded" + GalleryScript ScriptType = "gallery" + DiagnosticScript ScriptType = "diagnostic" + CommandIdScript ScriptType = "commandId" + NoneScript ScriptType = "none" +) + // Gets the InstallAsService field from the RunCommand's properties func (s HandlerSettings) InstallAsService() bool { return s.PublicSettings.InstallAsService @@ -28,7 +46,7 @@ func (s HandlerSettings) CommandId() string { return s.PublicSettings.Source.CommandId // Only applicable when the ScriptType is a CommandId. } -func (s HandlerSettings) ScriptType() types.ScriptType { +func (s HandlerSettings) ScriptType() ScriptType { return s.PublicSettings.Source.ScriptType } @@ -158,10 +176,10 @@ type RunCommandManagedIdentity struct { } type ScriptSource struct { - Script string `json:"script"` - ScriptURI string `json:"scriptUri"` - CommandId string `json:"commandId"` - ScriptType types.ScriptType `json:"scriptType"` + Script string `json:"script"` + ScriptURI string `json:"scriptUri"` + CommandId string `json:"commandId"` + ScriptType ScriptType `json:"scriptType"` } type ParameterDefinition struct { From 569809f57ae881c8222896142adeaf1a8c8132d1 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Mon, 20 Apr 2026 21:15:04 +0000 Subject: [PATCH 05/18] modified ut format + names --- .../extensionpolicysettingsrc/types_test.go | 105 +++++++----------- 1 file changed, 40 insertions(+), 65 deletions(-) diff --git a/internal/extensionpolicysettingsrc/types_test.go b/internal/extensionpolicysettingsrc/types_test.go index efb4515..618018d 100644 --- a/internal/extensionpolicysettingsrc/types_test.go +++ b/internal/extensionpolicysettingsrc/types_test.go @@ -4,19 +4,14 @@ import ( "testing" "github.com/Azure/run-command-handler-linux/internal/handlersettings" + "github.com/stretchr/testify/require" ) func TestTypeDefinitions_AreStable(t *testing.T) { t.Run("file type values are stable", func(t *testing.T) { - if got := string(All); got != "all" { - t.Fatalf("All = %q, want %q", got, "all") - } - if got := string(NoFiles); got != "none" { - t.Fatalf("NoFiles = %q, want %q", got, "none") - } - if got := string(Scripts); got != "scripts" { - t.Fatalf("Scripts = %q, want %q", got, "scripts") - } + require.Equal(t, "all", string(All)) + require.Equal(t, "none", string(NoFiles)) + require.Equal(t, "scripts", string(Scripts)) }) t.Run("allowed script flag values are stable", func(t *testing.T) { @@ -35,9 +30,9 @@ func TestTypeDefinitions_AreStable(t *testing.T) { } for _, tt := range tests { - if tt.got != tt.want { - t.Fatalf("%s = %d, want %d", tt.name, tt.got, tt.want) - } + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.got) + }) } }) @@ -56,9 +51,9 @@ func TestTypeDefinitions_AreStable(t *testing.T) { } for _, tt := range tests { - if tt.got != tt.want { - t.Fatalf("%s = %q, want %q", tt.name, tt.got, tt.want) - } + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.got) + }) } }) } @@ -76,12 +71,12 @@ func TestStringToAllowedScriptTypeFlag(t *testing.T) { want: Inline, }, { - name: "inline plus gallery", + name: "inline, gallery", input: "inline,gallery", want: Inline | Gallery, }, { - name: "allowed command id plus gallery plus inline", + name: "allowed command ID, gallery, inline", input: "allowedcommandid,gallery,inline", want: AllowedCommandId | Gallery | Inline, }, @@ -96,7 +91,7 @@ func TestStringToAllowedScriptTypeFlag(t *testing.T) { want: AllowAll, }, { - name: "whitespace and capitalization", + name: "whitespace and capitalization test", input: " InLiNe , GALLERY , allowedCommandId ", want: Inline | Gallery | AllowedCommandId, }, @@ -112,21 +107,13 @@ func TestStringToAllowedScriptTypeFlag(t *testing.T) { got, err := StringToAllowedScriptTypeFlag(tt.input) if tt.wantErr != "" { - if err == nil { - t.Fatalf("expected error %q, got nil", tt.wantErr) - } - if err.Error() != tt.wantErr { - t.Fatalf("error = %q, want %q", err.Error(), tt.wantErr) - } + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) return } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != tt.want { - t.Fatalf("got %d, want %d", got, tt.want) - } + require.NoError(t, err) + require.Equal(t, tt.want, got) }) } } @@ -148,14 +135,14 @@ func TestValidateFormat(t *testing.T) { }, }, { - name: "invalid limit scripts value", + name: "invalid value for limit scripts", input: RCv2ExtensionPolicySettings{ LimitScripts: "inline,notARealType", }, wantErr: "at least one of the values in LimitScripts is not a valid script type: inline,notARealType", }, { - name: "downloaded allowlist present but downloaded blocked", + name: "downloaded allowlist present, but downloaded scripts are blocked", input: RCv2ExtensionPolicySettings{ LimitScripts: "inline,gallery", DownloadedScriptsAllowlist: []string{"hash1"}, @@ -163,7 +150,7 @@ func TestValidateFormat(t *testing.T) { wantErr: "DownloadedScriptsAllowlist not empty, but LimitScripts does not allow 'downloaded' scripts", }, { - name: "command id allowlist present but command id blocked", + name: "command ID allowlist present, but command IDs are blocked", input: RCv2ExtensionPolicySettings{ LimitScripts: "inline,gallery", CommandIdAllowlist: []string{"cmd1"}, @@ -177,18 +164,12 @@ func TestValidateFormat(t *testing.T) { err := tt.input.ValidateFormat() if tt.wantErr != "" { - if err == nil { - t.Fatalf("expected error %q, got nil", tt.wantErr) - } - if err.Error() != tt.wantErr { - t.Fatalf("error = %q, want %q", err.Error(), tt.wantErr) - } + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) return } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoError(t, err) }) } } @@ -200,45 +181,45 @@ func TestCompareScriptTypeToAllowedScriptType(t *testing.T) { allowed AllowedScriptTypeFlag wantErr string }{ - // { - // name: "none allowed gallery denied", - // scriptType: handlersettings.GalleryScript, - // allowed: AllowedScriptNone, - // wantErr: "gallery scripts are not allowed by policy", - // }, { - name: "allow all inline", + name: "none allowed, gallery denied", + scriptType: handlersettings.GalleryScript, + allowed: AllowedScriptNone, + wantErr: "gallery scripts are not allowed by policy", + }, + { + name: "allow all, allow inline", scriptType: handlersettings.InlineScript, allowed: AllowAll, }, { - name: "allow all downloaded", + name: "allow all, allow downloaded", scriptType: handlersettings.DownloadedScript, allowed: AllowAll, }, { - name: "allow all gallery", + name: "allow all, allow gallery", scriptType: handlersettings.GalleryScript, allowed: AllowAll, }, { - name: "allow all diagnostic", + name: "allow all, allow diagnostic", scriptType: handlersettings.DiagnosticScript, allowed: AllowAll, }, { - name: "allow all command id", + name: "allow all, allow command id", scriptType: handlersettings.CommandIdScript, allowed: AllowAll, }, { - name: "diagnostic only inline denied", + name: "diagnostic only, inline denied", scriptType: handlersettings.InlineScript, allowed: Diagnostic, wantErr: "inline scripts are not allowed by policy", }, { - name: "allowed downloaded permits downloaded", + name: "allowed downloaded, allow downloaded", scriptType: handlersettings.DownloadedScript, allowed: AllowedDownloaded, }, @@ -249,7 +230,7 @@ func TestCompareScriptTypeToAllowedScriptType(t *testing.T) { wantErr: "unknown script type: made-up", }, { - name: "none script currently treated as unknown", + name: "'none' script currently treated as unknown", scriptType: handlersettings.NoneScript, allowed: AllowAll, wantErr: "unknown script type: none", @@ -261,18 +242,12 @@ func TestCompareScriptTypeToAllowedScriptType(t *testing.T) { err := CompareScriptTypeToAllowedScriptType(tt.scriptType, tt.allowed) if tt.wantErr != "" { - if err == nil { - t.Fatalf("expected error %q, got nil", tt.wantErr) - } - if err.Error() != tt.wantErr { - t.Fatalf("error = %q, want %q", err.Error(), tt.wantErr) - } + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) return } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoError(t, err) }) } } From 26b2525913e2c3a93c91076f6da9c8685fbca0c6 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Mon, 20 Apr 2026 21:52:44 +0000 Subject: [PATCH 06/18] del comment --- internal/handlersettings/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/handlersettings/types.go b/internal/handlersettings/types.go index 175aa66..4d68321 100644 --- a/internal/handlersettings/types.go +++ b/internal/handlersettings/types.go @@ -96,7 +96,7 @@ func (s HandlerSettings) ReadArtifacts() ([]UnifiedArtifact, error) { func (s HandlerSettings) validate() error { // If installAsService is false, then the source has to be specified if !s.PublicSettings.InstallAsService { - if s.PublicSettings.Source == nil || (s.PublicSettings.Source.Script == "") == (s.PublicSettings.Source.ScriptURI == "") { // Lourdes: check here also for scriptType? + if s.PublicSettings.Source == nil || (s.PublicSettings.Source.Script == "") == (s.PublicSettings.Source.ScriptURI == "") { return errSourceNotSpecified } } From 3d47105410db94159c0d794c052bfda250bee752 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Tue, 2 Jun 2026 17:55:06 +0000 Subject: [PATCH 07/18] fixed passing by value vs ref bug and added UTs --- internal/cmds/cmds.go | 2 +- internal/cmds/cmds_test.go | 77 ++++++++++++ .../extensionpolicysettingsrc.go | 38 +++--- .../extensionpolicysettingsrc_test.go | 115 +++++++++--------- internal/extensionpolicysettingsrc/types.go | 8 +- 5 files changed, 155 insertions(+), 85 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 8af04ed..cf03742 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -221,7 +221,7 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma var rceps *extensionpolicysettingsrc.RCv2ExtensionPolicySettings if _, err := os.Stat(policyPath); err == nil { - err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr, policyPath, rceps) + ExtensionPolicyManagerPtr, rceps, err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(policyPath) if err != nil { return "", "", errors.Wrap(err, "failed to initialize extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed } diff --git a/internal/cmds/cmds_test.go b/internal/cmds/cmds_test.go index 922876b..27cc471 100755 --- a/internal/cmds/cmds_test.go +++ b/internal/cmds/cmds_test.go @@ -1,6 +1,8 @@ package commands import ( + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "io/ioutil" @@ -17,6 +19,7 @@ import ( "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/internal/extensionpolicysettingsrc" "github.com/Azure/run-command-handler-linux/internal/files" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/Azure/run-command-handler-linux/internal/settings" @@ -1445,3 +1448,77 @@ func mustReadFile(t *testing.T, p string) string { } return string(b) } + +// Test_downloadScript_BlockedByAllowlist verifies that downloadScript returns an error +// when the policy allows downloaded scripts (alloweddownloaded) but the script's +// SHA256 hash is not in the DownloadedScriptsAllowlist. +func Test_downloadScript_BlockedByAllowlist(t *testing.T) { + dir, err := ioutil.TempDir("", "") + require.Nil(t, err) + defer os.RemoveAll(dir) + + scriptContent := []byte("#!/bin/bash\necho hello\n") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write(scriptContent) + })) + defer srv.Close() + + policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ + LimitScripts: "alloweddownloaded", + // A wrong hash — the script's actual hash is not this. + DownloadedScriptsAllowlist: []string{"0000000000000000000000000000000000000000000000000000000000000000"}, + } + + _, err = downloadScript(log.NewContext(log.NewNopLogger()), + dir, + &handlersettings.HandlerSettings{ + PublicSettings: handlersettings.PublicSettings{ + Source: &handlersettings.ScriptSource{ScriptURI: srv.URL + "/script.sh"}, + }, + }, + policy, + ) + require.Error(t, err) + require.Contains(t, err.Error(), "blocked by policy") + require.Contains(t, err.Error(), "item is not in the allowlist") +} + +// Test_downloadScript_AllowedByAllowlist verifies that downloadScript succeeds +// when the policy allows downloaded scripts and the script's SHA256 hash IS +// present in the DownloadedScriptsAllowlist. +func Test_downloadScript_AllowedByAllowlist(t *testing.T) { + dir, err := os.MkdirTemp("", "") + require.Nil(t, err) + defer os.RemoveAll(dir) + + // Content uses Unix LF only and has no BOM, so PostProcessFile leaves bytes + // unchanged, making the pre-computed hash match the on-disk file hash. + scriptContent := []byte("#!/bin/bash\necho hello\n") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write(scriptContent) + })) + defer srv.Close() + + // Compute the SHA256 hash that ValidateFileHashInAllowlist will compare against. + h := sha256.New() + h.Write(scriptContent) + correctHash := hex.EncodeToString(h.Sum(nil)) + + policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ + LimitScripts: "alloweddownloaded", + DownloadedScriptsAllowlist: []string{correctHash}, + } + + _, err = downloadScript(log.NewContext(log.NewNopLogger()), + dir, + &handlersettings.HandlerSettings{ + PublicSettings: handlersettings.PublicSettings{ + Source: &handlersettings.ScriptSource{ScriptURI: srv.URL + "/script.sh"}, + }, + }, + policy, + ) + require.NoError(t, err) +} diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go index 7a4bf3d..740a256 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go @@ -9,25 +9,26 @@ import ( "github.com/pkg/errors" ) -func InitializeExtensionPolicySettings(ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], - policyPath string, - rceps *RCv2ExtensionPolicySettings) error { +func InitializeExtensionPolicySettings(policyPath string) (*extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], *RCv2ExtensionPolicySettings, error) { + var ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] + var rceps *RCv2ExtensionPolicySettings + ExtensionPolicyManagerPtr, err := extensionpolicysettings.NewExtensionPolicySettingsManager[RCv2ExtensionPolicySettings](policyPath) if err != nil { - return errors.Wrap(err, "failed to create extension policy settings manager") + return nil, nil, errors.Wrap(err, "failed to create extension policy settings manager") } err = ExtensionPolicyManagerPtr.LoadExtensionPolicySettings() if err != nil { - return errors.Wrap(err, "failed to load extension policy settings") + return nil, nil, errors.Wrap(err, "failed to load extension policy settings") } else { rceps, err = ExtensionPolicyManagerPtr.GetSettings() if err != nil { - return errors.Wrap(err, "failed to get extension policy settings") + return nil, nil, errors.Wrap(err, "failed to get extension policy settings") } } - return nil + return ExtensionPolicyManagerPtr, rceps, nil } func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { @@ -47,9 +48,9 @@ func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.Handl return err } } - if policy.DisableOutputBlobs { - ValidateOutputBlob(settings, policy) - } + + // TO-DO: Validate Disable Outputblob and RequireSigning once those features are implemented for RCv2. + return nil } @@ -71,7 +72,11 @@ func ValidateCommandId(settings *handlersettings.HandlerSettings, policy *RCv2Ex // if list is empty, all commandIds are allowed return nil } - return extensionpolicysettings.ValidateValueInAllowlist(settingsCommandId, allowedCommandIds) + err := extensionpolicysettings.ValidateValueInAllowlist(settingsCommandId, allowedCommandIds) + if err != nil { + return errors.Wrapf(err, "command ID %s is not allowed by policy", settingsCommandId) + } + return nil } func ValidateRunAsUser(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { @@ -83,14 +88,3 @@ func ValidateRunAsUser(settings *handlersettings.HandlerSettings, policy *RCv2Ex } return nil } - -func ValidateOutputBlob(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) { - if policy.DisableOutputBlobs { - // Log a warning that output blobs are disabled by policy. The command will still execute, but no output blobs will be created. - if settings.OutputBlobURI != "" { - fmt.Println("Warning: Output blobs are disabled by policy. The provided output blob URI will be ignored and no output blobs will be created for this command.") - } else { - fmt.Println("Warning: Output blobs are disabled by policy. No output blobs will be created for this command.") - } - } -} diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go index 0c50d68..3354f7c 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go @@ -6,7 +6,6 @@ import ( "path/filepath" "testing" - "github.com/Azure/azure-extension-platform/pkg/extensionpolicysettings" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/stretchr/testify/require" ) @@ -44,10 +43,7 @@ func captureStdout(t *testing.T, fn func()) string { } func TestInitializeExtensionPolicySettings_InvalidPath_ReturnsError(t *testing.T) { - var mgr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] - out := &RCv2ExtensionPolicySettings{} - - err := InitializeExtensionPolicySettings(mgr, "/definitely/not/found/policy.json", out) + _, _, err := InitializeExtensionPolicySettings("/definitely/not/found/policy.json") require.Error(t, err) require.Contains(t, err.Error(), "failed to") } @@ -60,10 +56,7 @@ func TestInitializeExtensionPolicySettings_ValidFile_ReturnsNil(t *testing.T) { err := os.WriteFile(policyPath, []byte("{}"), 0600) require.NoError(t, err) - var mgr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] - out := &RCv2ExtensionPolicySettings{} - - err = InitializeExtensionPolicySettings(mgr, policyPath, out) + _, _, err = InitializeExtensionPolicySettings(policyPath) require.NoError(t, err) } @@ -75,17 +68,16 @@ func TestInitializeExtensionPolicySettings_CurrentBehavior_DoesNotPopulateOutput err := os.WriteFile(policyPath, []byte(payload), 0600) require.NoError(t, err) - var mgr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] out := &RCv2ExtensionPolicySettings{} - err = InitializeExtensionPolicySettings(mgr, policyPath, out) + _, out, err = InitializeExtensionPolicySettings(policyPath) require.NoError(t, err) - // Documents current implementation behavior (pointer reassignment inside function). - require.Equal(t, "", out.LimitScripts) - require.Equal(t, "", out.RunAsUser) + require.Equal(t, "inline", out.LimitScripts) + require.Equal(t, "alice", out.RunAsUser) } +// Test that validation passes and fails as expected. func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { t.Run("nil policy", func(t *testing.T) { settings := makeSettings(handlersettings.InlineScript, "", "", "") @@ -94,6 +86,8 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { require.Contains(t, err.Error(), "no policy provided") }) + // This test mimicks running an inline script, but policy only allows gallery scripts. + // Validation fails. t.Run("script type blocked by policy", func(t *testing.T) { settings := makeSettings(handlersettings.InlineScript, "", "", "") policy := &RCv2ExtensionPolicySettings{ @@ -105,7 +99,9 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { require.Contains(t, err.Error(), "script type inline is not allowed by policy") }) - t.Run("command id not in allowlist", func(t *testing.T) { + // This test mimicks running a commandId that is not in the allowlist. + // Additionally, only commandId types are allowed. + t.Run("command ID not in allowlist", func(t *testing.T) { settings := makeSettings(handlersettings.CommandIdScript, "restartVM", "", "") policy := &RCv2ExtensionPolicySettings{ LimitScripts: "allowedcommandid", @@ -128,7 +124,33 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { require.Contains(t, err.Error(), "does not match") }) - t.Run("all checks pass", func(t *testing.T) { + t.Run("enforce limitScripts must be set. If not set, all commands fail", func(t *testing.T) { + settings := makeSettings(handlersettings.CommandIdScript, "safeCommand", " Alice ", "https://example/blob") + policy := &RCv2ExtensionPolicySettings{ + LimitScripts: "", + CommandIdAllowlist: []string{"safeCommand"}, + RunAsUser: "Alice", + DisableOutputBlobs: true, + } + + err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + require.Contains(t, err.Error(), "script type commandId is not allowed by policy") + }) + + t.Run("all checks pass commandId", func(t *testing.T) { + settings := makeSettings(handlersettings.CommandIdScript, "safeCommand", " Alice ", "https://example/blob") + policy := &RCv2ExtensionPolicySettings{ + LimitScripts: "allowall", + CommandIdAllowlist: []string{"safeCommand"}, + RunAsUser: "alice", + DisableOutputBlobs: true, + } + + err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + require.NoError(t, err) + }) + + t.Run("all checks pass commandId", func(t *testing.T) { settings := makeSettings(handlersettings.CommandIdScript, "safeCommand", " Alice ", "https://example/blob") policy := &RCv2ExtensionPolicySettings{ LimitScripts: "allowall", @@ -140,6 +162,19 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) require.NoError(t, err) }) + + t.Run("all checks pass downloadedScript", func(t *testing.T) { + settings := makeSettings(handlersettings.DownloadedScript, "safeCommand", " Alice ", "https://example/blob") + policy := &RCv2ExtensionPolicySettings{ + LimitScripts: "alloweddownloaded", + CommandIdAllowlist: []string{"safeCommand"}, + RunAsUser: "alice", + DisableOutputBlobs: true, + } + + err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + require.NoError(t, err) + }) } func TestValidateScriptTypeAgainstPolicy(t *testing.T) { @@ -154,7 +189,8 @@ func TestValidateScriptTypeAgainstPolicy(t *testing.T) { require.Contains(t, err.Error(), "script type gallery is not allowed by policy") }) - t.Run("invalid policy token currently treated as blocked", func(t *testing.T) { + // This tests edge case where policy has an invalid script type token. + t.Run("invalid policy token is treated as blocked", func(t *testing.T) { err := ValidateScriptTypeAgainstPolicy(handlersettings.InlineScript, "notARealScriptType") require.Error(t, err) require.Contains(t, err.Error(), "script type inline is not allowed by policy") @@ -186,7 +222,8 @@ func TestValidateCommandId(t *testing.T) { CommandIdAllowlist: []string{"safeCommand", "other"}, } err := ValidateCommandId(settings, policy) - require.Error(t, err) + require.Contains(t, err.Error(), "command ID restartVM is not allowed by policy") + require.Contains(t, err.Error(), "item is not in the allowlist") }) } @@ -207,46 +244,6 @@ func TestValidateRunAsUser(t *testing.T) { } err := ValidateRunAsUser(settings, policy) require.Error(t, err) - require.Contains(t, err.Error(), "does not match") - }) -} - -func TestValidateOutputBlob(t *testing.T) { - t.Run("policy does not disable output blobs prints nothing", func(t *testing.T) { - settings := makeSettings(handlersettings.InlineScript, "", "", "https://example/blob") - policy := &RCv2ExtensionPolicySettings{ - DisableOutputBlobs: false, - } - - out := captureStdout(t, func() { - ValidateOutputBlob(settings, policy) - }) - require.Equal(t, "", out) - }) - - t.Run("disabled with output blob uri prints ignore warning", func(t *testing.T) { - settings := makeSettings(handlersettings.InlineScript, "", "", "https://example/blob") - policy := &RCv2ExtensionPolicySettings{ - DisableOutputBlobs: true, - } - - out := captureStdout(t, func() { - ValidateOutputBlob(settings, policy) - }) - require.Contains(t, out, "Output blobs are disabled by policy") - require.Contains(t, out, "provided output blob URI will be ignored") - }) - - t.Run("disabled without output blob uri prints no blob warning", func(t *testing.T) { - settings := makeSettings(handlersettings.InlineScript, "", "", "") - policy := &RCv2ExtensionPolicySettings{ - DisableOutputBlobs: true, - } - - out := captureStdout(t, func() { - ValidateOutputBlob(settings, policy) - }) - require.Contains(t, out, "Output blobs are disabled by policy") - require.Contains(t, out, "No output blobs will be created") + require.Contains(t, err.Error(), "RunAsUser 'bob' in settings does not match RunAsUser 'alice' in policy") }) } diff --git a/internal/extensionpolicysettingsrc/types.go b/internal/extensionpolicysettingsrc/types.go index 64c73a9..f6eea20 100644 --- a/internal/extensionpolicysettingsrc/types.go +++ b/internal/extensionpolicysettingsrc/types.go @@ -86,6 +86,7 @@ func (rceps RCv2ExtensionPolicySettings) ValidateFormat() error { flag, err := StringToAllowedScriptTypeFlag(string(rceps.LimitScripts)) // Requirements: // 1. If RequireSigning is not "none", FileRootCert must be present and non-empty. + // TO-DO: implement RequireSigning and FileRootCert validation once signature verification is implemented for RCv2. // 2. LimitScripts must be a valid AllowedScriptType value. so map/check the value to the AllowedScriptTypeFlag bitmask. if rceps.LimitScripts != "" { if err != nil { @@ -107,9 +108,10 @@ func (rceps RCv2ExtensionPolicySettings) ValidateFormat() error { return nil } -// This function compares a script type (of type ScriptType, defined in this file) to the allowed script types -// (of type AllowedScriptTypeFlag, also defined in this file) listed in the policy. These values and mappings -// are specific to Run Command, hence why they are defined here and not in the shared library. +// This function compares a script type (of string type ScriptType, defined in this file) to the allowed script types +// (of type AllowedScriptTypeFlag, also defined in this file) listed in the policy. +// Depending on the string case (the value of scriptType), it checks if the corresponding bit is enabled in the allowed script types bitmask. +// These values and mappings are specific to Run Command, hence why they are defined here and not in the shared library. func CompareScriptTypeToAllowedScriptType(scriptType handlersettings.ScriptType, allowedScriptTypes AllowedScriptTypeFlag) error { switch scriptType { case handlersettings.InlineScript: From 6577857175b188a53f125d4c3e1fb17973c114c2 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Mon, 22 Jun 2026 18:31:45 +0000 Subject: [PATCH 08/18] addressing comments --- internal/cmds/cmds.go | 10 +-- internal/cmds/cmds_test.go | 2 +- internal/constants/constants.go | 5 ++ internal/constants/exitcodes.go | 12 ++-- .../extensionpolicysettingsrc.go | 54 ++++++++------- .../extensionpolicysettingsrc_test.go | 65 +++++++------------ 6 files changed, 73 insertions(+), 75 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index cf03742..8b6d959 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -221,9 +221,9 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma var rceps *extensionpolicysettingsrc.RCv2ExtensionPolicySettings if _, err := os.Stat(policyPath); err == nil { - ExtensionPolicyManagerPtr, rceps, err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(policyPath) + ExtensionPolicyManagerPtr, rceps, err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ctx, policyPath) if err != nil { - return "", "", errors.Wrap(err, "failed to initialize extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed + return "", "", errors.Wrap(err, "failed in enable to initialize extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed } ctx.Log("message", "successfully initialized extension policy settings") } else if os.IsNotExist(err) { @@ -235,15 +235,15 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma // Validate handler settings against policy settings. if ExtensionPolicyManagerPtr != nil && rceps != nil { - if err = extensionpolicysettingsrc.InitialValidateHandlerSettingsAgainstPolicy(&cfg, rceps); err != nil { - return "", "", err, constants.ExitCode_HandlerSettingsViolatePolicy + if err = extensionpolicysettingsrc.ValidateHandlerSettingsAgainstPolicy(ctx, &cfg, rceps); err != nil { + return "", "", err, constants.ExitCode_HandlerSettingsViolateExtensionPolicy } } dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) scriptFilePath, err := downloadScript(ctx, dir, &cfg, rceps) if err != nil && errors.Is(err, extensionerrors.ErrItemNotInAllowlist) { - return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist"), constants.ExitCode_DownloadedScriptBlockedByPolicy + return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist"), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy } if err != nil { errMessage := fmt.Sprintf("Failed to download script: %v due to: %v", download.GetUriForLogging(cfg.ScriptURI()), err) diff --git a/internal/cmds/cmds_test.go b/internal/cmds/cmds_test.go index 27cc471..ca54970 100755 --- a/internal/cmds/cmds_test.go +++ b/internal/cmds/cmds_test.go @@ -1466,7 +1466,7 @@ func Test_downloadScript_BlockedByAllowlist(t *testing.T) { policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ LimitScripts: "alloweddownloaded", - // A wrong hash — the script's actual hash is not this. + // A mismatch hash DownloadedScriptsAllowlist: []string{"0000000000000000000000000000000000000000000000000000000000000000"}, } diff --git a/internal/constants/constants.go b/internal/constants/constants.go index b7aacbf..c54965b 100755 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -80,4 +80,9 @@ const ( // The name of the policy file the extension uses to validate the command + scripts before execution. PolicyFileName = "waagent_runtime_policy.json" + + // Name of our ICM queue for service errors. Right now, this is only displayed in logs for extension policy errors. + ICMQueueName = "AzureRT\\Extensions" + + ContactICMForServiceErrorsMessage = "Contact ICM team " + ICMQueueName + " for this service error" ) diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index 9181572..a05a864 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -35,13 +35,11 @@ const ( ExitCode_ImmediateTaskTimeout = -222 ExitCode_ImmediateTaskFailed = -223 ExitCode_CouldNotRehydrateMrSeq = -224 - - // Extension Policy Settings Errors (-300s): - ExitCode_LoadExtensionPolicySettingsFailed = -300 - ExitCode_GetExtensionPolicySettingsFailed = -301 - ExitCode_ExtensionPolicyInvalid = -302 - ExitCode_HandlerSettingsViolatePolicy = -303 - ExitCode_DownloadedScriptBlockedByPolicy = -304 + ExitCode_LoadExtensionPolicySettingsFailed = -300 + ExitCode_GetExtensionPolicySettingsFailed = -301 + ExitCode_ExtensionPolicyInvalid = -302 + ExitCode_HandlerSettingsViolateExtensionPolicy = -303 + ExitCode_DownloadedScriptBlockedByExtensionPolicy = -304 // Unknown errors (-400s): ) diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go index 740a256..8ef4890 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go @@ -5,46 +5,51 @@ import ( "strings" "github.com/Azure/azure-extension-platform/pkg/extensionpolicysettings" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" + "github.com/go-kit/kit/log" "github.com/pkg/errors" ) -func InitializeExtensionPolicySettings(policyPath string) (*extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], *RCv2ExtensionPolicySettings, error) { - var ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings] - var rceps *RCv2ExtensionPolicySettings - - ExtensionPolicyManagerPtr, err := extensionpolicysettings.NewExtensionPolicySettingsManager[RCv2ExtensionPolicySettings](policyPath) +func InitializeExtensionPolicySettings(ctx *log.Context, policyPath string) (*extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], *RCv2ExtensionPolicySettings, error) { + extensionPolicyManager, err := extensionpolicysettings.NewExtensionPolicySettingsManager[RCv2ExtensionPolicySettings](policyPath) if err != nil { - return nil, nil, errors.Wrap(err, "failed to create extension policy settings manager") + err = errors.Wrap(err, "failed to create extension policy settings manager") + ctx.Log("message", "failed to create extension policy settings manager. "+constants.ContactICMForServiceErrorsMessage, "error", err, "policyPath", policyPath) + return nil, nil, err } - err = ExtensionPolicyManagerPtr.LoadExtensionPolicySettings() + err = extensionPolicyManager.LoadExtensionPolicySettings() if err != nil { - return nil, nil, errors.Wrap(err, "failed to load extension policy settings") - } else { - rceps, err = ExtensionPolicyManagerPtr.GetSettings() + err = errors.Wrap(err, "failed to load extension policy settings") + ctx.Log("message", "failed to load extension policy settings. "+constants.ContactICMForServiceErrorsMessage, "error", err, "policyPath", policyPath) + return nil, nil, err + } - if err != nil { - return nil, nil, errors.Wrap(err, "failed to get extension policy settings") - } + rceps, err := extensionPolicyManager.GetSettings() //rceps is the pointer to the actual policy struct + if err != nil { + err = errors.Wrap(err, "failed to get extension policy settings after loading") + ctx.Log("message", "failed to get extension policy settings. "+constants.ContactICMForServiceErrorsMessage, "error", err, "policyPath", policyPath) + return nil, nil, err } - return ExtensionPolicyManagerPtr, rceps, nil + return extensionPolicyManager, rceps, nil } -func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { +func ValidateHandlerSettingsAgainstPolicy(ctx *log.Context, settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { if policy == nil { + ctx.Log("message", "no policy provided for extension policy settings") return fmt.Errorf("no policy provided") } - if err := ValidateScriptTypeAgainstPolicy(settings.ScriptType(), policy.LimitScripts); err != nil { + if err := ValidateScriptTypeAgainstPolicy(ctx, settings.ScriptType(), policy.LimitScripts); err != nil { return err } if settings.ScriptType() == handlersettings.CommandIdScript { - if err := ValidateCommandId(settings, policy); err != nil { + if err := ValidateCommandId(ctx, settings, policy); err != nil { return err } } if policy.RunAsUser != "" { - if err := ValidateRunAsUser(settings, policy); err != nil { + if err := ValidateRunAsUser(ctx, settings, policy); err != nil { return err } } @@ -54,37 +59,42 @@ func InitialValidateHandlerSettingsAgainstPolicy(settings *handlersettings.Handl return nil } -func ValidateScriptTypeAgainstPolicy(scriptType handlersettings.ScriptType, allowedScriptTypesString string) error { +func ValidateScriptTypeAgainstPolicy(ctx *log.Context, scriptType handlersettings.ScriptType, allowedScriptTypesString string) error { allowedScriptTypes, _ := StringToAllowedScriptTypeFlag(allowedScriptTypesString) // Compare the script type of the command with the allowed script types in the policy. err := CompareScriptTypeToAllowedScriptType(scriptType, allowedScriptTypes) if err != nil { + ctx.Log("message", "script type not allowed by policy", "error", err, "scriptType", scriptType) return errors.Wrapf(err, "script type %s is not allowed by policy", scriptType) } return nil } -func ValidateCommandId(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { +func ValidateCommandId(ctx *log.Context, settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { settingsCommandId := settings.CommandId() allowedCommandIds := policy.CommandIdAllowlist if len(allowedCommandIds) == 0 { // if list is empty, all commandIds are allowed + ctx.Log("message", "allowedCommandID list empty, allowing all commands") return nil } err := extensionpolicysettings.ValidateValueInAllowlist(settingsCommandId, allowedCommandIds) if err != nil { + ctx.Log("message", "command ID is not allowed by policy", "error", err, "commandId", settingsCommandId) return errors.Wrapf(err, "command ID %s is not allowed by policy", settingsCommandId) } return nil } -func ValidateRunAsUser(settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { +func ValidateRunAsUser(ctx *log.Context, settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { settingsRunAsUser := strings.ToLower(strings.TrimSpace(settings.RunAsUser)) policyRunAsUser := strings.ToLower(strings.TrimSpace(policy.RunAsUser)) if strings.Compare(settingsRunAsUser, policyRunAsUser) != 0 { - return fmt.Errorf("RunAsUser '%s' in settings does not match RunAsUser '%s' in policy", settingsRunAsUser, policyRunAsUser) + err := fmt.Errorf("runAsUser '%s' in settings does not match runAsUser '%s' in policy", settingsRunAsUser, policyRunAsUser) + ctx.Log("message", "runAsUser settings does not match runAsUser in policy", "error", err, "settingsRunAsUser", settingsRunAsUser, "policyRunAsUser", policyRunAsUser) + return err } return nil } diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go index 3354f7c..c7c9e2d 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go @@ -1,12 +1,12 @@ package extensionpolicysettingsrc import ( - "io" "os" "path/filepath" "testing" "github.com/Azure/run-command-handler-linux/internal/handlersettings" + "github.com/go-kit/kit/log" "github.com/stretchr/testify/require" ) @@ -23,27 +23,8 @@ func makeSettings(scriptType handlersettings.ScriptType, commandID string, runAs } } -func captureStdout(t *testing.T, fn func()) string { - t.Helper() - - old := os.Stdout - r, w, err := os.Pipe() - require.NoError(t, err) - - os.Stdout = w - fn() - _ = w.Close() - os.Stdout = old - - out, err := io.ReadAll(r) - require.NoError(t, err) - _ = r.Close() - - return string(out) -} - func TestInitializeExtensionPolicySettings_InvalidPath_ReturnsError(t *testing.T) { - _, _, err := InitializeExtensionPolicySettings("/definitely/not/found/policy.json") + _, _, err := InitializeExtensionPolicySettings(nopCtx(), "/definitely/not/found/policy.json") require.Error(t, err) require.Contains(t, err.Error(), "failed to") } @@ -56,7 +37,7 @@ func TestInitializeExtensionPolicySettings_ValidFile_ReturnsNil(t *testing.T) { err := os.WriteFile(policyPath, []byte("{}"), 0600) require.NoError(t, err) - _, _, err = InitializeExtensionPolicySettings(policyPath) + _, _, err = InitializeExtensionPolicySettings(nopCtx(), policyPath) require.NoError(t, err) } @@ -70,7 +51,7 @@ func TestInitializeExtensionPolicySettings_CurrentBehavior_DoesNotPopulateOutput out := &RCv2ExtensionPolicySettings{} - _, out, err = InitializeExtensionPolicySettings(policyPath) + _, out, err = InitializeExtensionPolicySettings(nopCtx(), policyPath) require.NoError(t, err) require.Equal(t, "inline", out.LimitScripts) @@ -81,7 +62,7 @@ func TestInitializeExtensionPolicySettings_CurrentBehavior_DoesNotPopulateOutput func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { t.Run("nil policy", func(t *testing.T) { settings := makeSettings(handlersettings.InlineScript, "", "", "") - err := InitialValidateHandlerSettingsAgainstPolicy(settings, nil) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, nil) require.Error(t, err) require.Contains(t, err.Error(), "no policy provided") }) @@ -94,7 +75,7 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { LimitScripts: "gallery", } - err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Error(t, err) require.Contains(t, err.Error(), "script type inline is not allowed by policy") }) @@ -108,7 +89,7 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { CommandIdAllowlist: []string{"safeCommand"}, } - err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Error(t, err) }) @@ -119,7 +100,7 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { RunAsUser: "alice", } - err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Error(t, err) require.Contains(t, err.Error(), "does not match") }) @@ -133,7 +114,7 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { DisableOutputBlobs: true, } - err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Contains(t, err.Error(), "script type commandId is not allowed by policy") }) @@ -146,7 +127,7 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { DisableOutputBlobs: true, } - err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.NoError(t, err) }) @@ -159,7 +140,7 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { DisableOutputBlobs: true, } - err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.NoError(t, err) }) @@ -172,26 +153,26 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { DisableOutputBlobs: true, } - err := InitialValidateHandlerSettingsAgainstPolicy(settings, policy) + err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.NoError(t, err) }) } func TestValidateScriptTypeAgainstPolicy(t *testing.T) { t.Run("allowed", func(t *testing.T) { - err := ValidateScriptTypeAgainstPolicy(handlersettings.InlineScript, "inline") + err := ValidateScriptTypeAgainstPolicy(nopCtx(), handlersettings.InlineScript, "inline") require.NoError(t, err) }) t.Run("blocked", func(t *testing.T) { - err := ValidateScriptTypeAgainstPolicy(handlersettings.GalleryScript, "inline") + err := ValidateScriptTypeAgainstPolicy(nopCtx(), handlersettings.GalleryScript, "inline") require.Error(t, err) require.Contains(t, err.Error(), "script type gallery is not allowed by policy") }) // This tests edge case where policy has an invalid script type token. t.Run("invalid policy token is treated as blocked", func(t *testing.T) { - err := ValidateScriptTypeAgainstPolicy(handlersettings.InlineScript, "notARealScriptType") + err := ValidateScriptTypeAgainstPolicy(nopCtx(), handlersettings.InlineScript, "notARealScriptType") require.Error(t, err) require.Contains(t, err.Error(), "script type inline is not allowed by policy") }) @@ -203,7 +184,7 @@ func TestValidateCommandId(t *testing.T) { policy := &RCv2ExtensionPolicySettings{ CommandIdAllowlist: nil, } - err := ValidateCommandId(settings, policy) + err := ValidateCommandId(nopCtx(), settings, policy) require.NoError(t, err) }) @@ -212,7 +193,7 @@ func TestValidateCommandId(t *testing.T) { policy := &RCv2ExtensionPolicySettings{ CommandIdAllowlist: []string{"safeCommand", "other"}, } - err := ValidateCommandId(settings, policy) + err := ValidateCommandId(nopCtx(), settings, policy) require.NoError(t, err) }) @@ -221,7 +202,7 @@ func TestValidateCommandId(t *testing.T) { policy := &RCv2ExtensionPolicySettings{ CommandIdAllowlist: []string{"safeCommand", "other"}, } - err := ValidateCommandId(settings, policy) + err := ValidateCommandId(nopCtx(), settings, policy) require.Contains(t, err.Error(), "command ID restartVM is not allowed by policy") require.Contains(t, err.Error(), "item is not in the allowlist") }) @@ -233,7 +214,7 @@ func TestValidateRunAsUser(t *testing.T) { policy := &RCv2ExtensionPolicySettings{ RunAsUser: "alice", } - err := ValidateRunAsUser(settings, policy) + err := ValidateRunAsUser(nopCtx(), settings, policy) require.NoError(t, err) }) @@ -242,8 +223,12 @@ func TestValidateRunAsUser(t *testing.T) { policy := &RCv2ExtensionPolicySettings{ RunAsUser: "alice", } - err := ValidateRunAsUser(settings, policy) + err := ValidateRunAsUser(nopCtx(), settings, policy) require.Error(t, err) - require.Contains(t, err.Error(), "RunAsUser 'bob' in settings does not match RunAsUser 'alice' in policy") + require.Contains(t, err.Error(), "runAsUser 'bob' in settings does not match runAsUser 'alice' in policy") }) } + +func nopCtx() *log.Context { + return log.NewContext(log.NewNopLogger()) +} From 71cfa1e67a747e595f052bd1aae21f315b70c4b2 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Mon, 22 Jun 2026 18:33:18 +0000 Subject: [PATCH 09/18] del unused exit code --- internal/constants/exitcodes.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index a05a864..c8562f5 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -36,7 +36,6 @@ const ( ExitCode_ImmediateTaskFailed = -223 ExitCode_CouldNotRehydrateMrSeq = -224 ExitCode_LoadExtensionPolicySettingsFailed = -300 - ExitCode_GetExtensionPolicySettingsFailed = -301 ExitCode_ExtensionPolicyInvalid = -302 ExitCode_HandlerSettingsViolateExtensionPolicy = -303 ExitCode_DownloadedScriptBlockedByExtensionPolicy = -304 From 094af6af65ebe75c3ce03717552bdd06ac7bbb78 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Mon, 22 Jun 2026 18:37:42 +0000 Subject: [PATCH 10/18] updated exit codes --- internal/constants/exitcodes.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index c8562f5..f91a653 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -5,9 +5,12 @@ const ( ExitCode_Okay = 0 // User errors (-100s): - ExitCode_ScriptBlobDownloadFailed = -100 - ExitCode_BlobCreateOrReplaceFailed = -101 - ExitCode_RunAsLookupUserFailed = -102 + ExitCode_ScriptBlobDownloadFailed = -100 + ExitCode_BlobCreateOrReplaceFailed = -101 + ExitCode_RunAsLookupUserFailed = -102 + ExitCode_HandlerSettingsViolateExtensionPolicy = -103 + ExitCode_DownloadedScriptBlockedByExtensionPolicy = -104 + ExitCode_ExtensionPolicyInvalid = -226 // Service Errors (-200s): ExitCode_CreateDataDirectoryFailed = -200 @@ -35,10 +38,5 @@ const ( ExitCode_ImmediateTaskTimeout = -222 ExitCode_ImmediateTaskFailed = -223 ExitCode_CouldNotRehydrateMrSeq = -224 - ExitCode_LoadExtensionPolicySettingsFailed = -300 - ExitCode_ExtensionPolicyInvalid = -302 - ExitCode_HandlerSettingsViolateExtensionPolicy = -303 - ExitCode_DownloadedScriptBlockedByExtensionPolicy = -304 - - // Unknown errors (-400s): + ExitCode_LoadExtensionPolicySettingsFailed = -225 ) From 38dd6baee5411342bfb99adb8d592c9a465c1fe1 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Mon, 22 Jun 2026 18:41:25 +0000 Subject: [PATCH 11/18] updated exit codes --- internal/cmds/cmds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 8b6d959..76b6125 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -230,7 +230,7 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma ctx.Log("message", "extension policy settings file does not exist. No policy applied.", "error", err) ExtensionPolicyManagerPtr = nil } else { - return "", "", errors.Wrap(err, "failed to stat extension policy settings file"), constants.ExitCode_LoadExtensionPolicySettingsFailed + return "", "", errors.Wrap(err, "failed to stat extension policy settings file in enable"), constants.ExitCode_LoadExtensionPolicySettingsFailed } // Validate handler settings against policy settings. From e1940fe696afb58a2f71537c9e74267b855f87dd Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Wed, 24 Jun 2026 20:21:21 +0000 Subject: [PATCH 12/18] added tests --- internal/cmds/cmds.go | 9 +- internal/cmds/cmds_test.go | 187 ++++++++++++++++-- internal/commandProcessor/commandProcessor.go | 8 +- 3 files changed, 184 insertions(+), 20 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 76b6125..5f7c179 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -216,25 +216,25 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma // Load extension policy settings. // If policy file exists, load the policy. If not, then don't load. - var ExtensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[extensionpolicysettingsrc.RCv2ExtensionPolicySettings] + var extensionPolicyManagerPtr *extensionpolicysettings.ExtensionPolicySettingsManager[extensionpolicysettingsrc.RCv2ExtensionPolicySettings] policyPath := filepath.Join(h.HandlerEnvironment.ConfigFolder, constants.PolicyFileName) var rceps *extensionpolicysettingsrc.RCv2ExtensionPolicySettings if _, err := os.Stat(policyPath); err == nil { - ExtensionPolicyManagerPtr, rceps, err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ctx, policyPath) + extensionPolicyManagerPtr, rceps, err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ctx, policyPath) if err != nil { return "", "", errors.Wrap(err, "failed in enable to initialize extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed } ctx.Log("message", "successfully initialized extension policy settings") } else if os.IsNotExist(err) { ctx.Log("message", "extension policy settings file does not exist. No policy applied.", "error", err) - ExtensionPolicyManagerPtr = nil + extensionPolicyManagerPtr = nil } else { return "", "", errors.Wrap(err, "failed to stat extension policy settings file in enable"), constants.ExitCode_LoadExtensionPolicySettingsFailed } // Validate handler settings against policy settings. - if ExtensionPolicyManagerPtr != nil && rceps != nil { + if extensionPolicyManagerPtr != nil && rceps != nil { if err = extensionpolicysettingsrc.ValidateHandlerSettingsAgainstPolicy(ctx, &cfg, rceps); err != nil { return "", "", err, constants.ExitCode_HandlerSettingsViolateExtensionPolicy } @@ -963,7 +963,6 @@ func runCmd(ctx *log.Context, dir string, scriptFilePath string, cfg *handlerset scenario = "public-scriptUri" } - // Filter the inline script type here. ctx.Log("event", "prepare command", "scriptFile", scriptFilePath) // We need to kill previous extension process if exists before starting a new one. diff --git a/internal/cmds/cmds_test.go b/internal/cmds/cmds_test.go index ca54970..e29ecc7 100755 --- a/internal/cmds/cmds_test.go +++ b/internal/cmds/cmds_test.go @@ -18,6 +18,7 @@ import ( "github.com/Azure/azure-extension-platform/pkg/extensionevents" "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/Azure/run-command-handler-linux/internal/commandProcessor" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/extensionpolicysettingsrc" "github.com/Azure/run-command-handler-linux/internal/files" @@ -1458,10 +1459,7 @@ func Test_downloadScript_BlockedByAllowlist(t *testing.T) { defer os.RemoveAll(dir) scriptContent := []byte("#!/bin/bash\necho hello\n") - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write(scriptContent) - })) + srv := make_server_with_content(scriptContent) defer srv.Close() policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ @@ -1495,16 +1493,11 @@ func Test_downloadScript_AllowedByAllowlist(t *testing.T) { // Content uses Unix LF only and has no BOM, so PostProcessFile leaves bytes // unchanged, making the pre-computed hash match the on-disk file hash. scriptContent := []byte("#!/bin/bash\necho hello\n") - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write(scriptContent) - })) + srv := make_server_with_content(scriptContent) defer srv.Close() // Compute the SHA256 hash that ValidateFileHashInAllowlist will compare against. - h := sha256.New() - h.Write(scriptContent) - correctHash := hex.EncodeToString(h.Sum(nil)) + correctHash := hash_bytes_256(scriptContent) policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ LimitScripts: "alloweddownloaded", @@ -1522,3 +1515,175 @@ func Test_downloadScript_AllowedByAllowlist(t *testing.T) { ) require.NoError(t, err) } + +func setupPolicyE2E(t *testing.T, dataDir, extName string, seqNum int, scriptURI string, treatFailureAsDeploymentFailure bool, policy *extensionpolicysettingsrc.RCv2ExtensionPolicySettings, +) types.HandlerEnvironment { + t.Helper() + configFolder := create_folder(t, dataDir, "config") + statusFolder := create_folder(t, dataDir, constants.StatusFileDirectory) + eventsFolder := create_folder(t, dataDir, constants.ExtensionEventsDirectory) + + fakeEnv := types.HandlerEnvironment{} + update_handler_env(&fakeEnv, statusFolder, configFolder, eventsFolder) + + // Write the extension .settings file (mirrors enable_extension), but with a + // downloaded-script source so the allowlist check applies. + settingsCommon := settings.SettingsCommon{ + ExtensionName: &extName, + ProtectedSettingsBase64: "", + SettingsCertThumbprint: "", + PublicSettings: map[string]interface{}{ + "source": map[string]interface{}{ + "scriptUri": scriptURI, + "scriptType": string(handlersettings.DownloadedScript), + }, + "treatFailureAsDeploymentFailure": treatFailureAsDeploymentFailure, + }, + } + handlerSettings := handlersettings.HandlerSettingsFile{ + RuntimeSettings: []handlersettings.RunTimeSettingsFile{ + {HandlerSettings: settingsCommon}, + }, + } + settingsFilePath := filepath.Join(configFolder, extName+"."+strconv.Itoa(seqNum)+".settings") + file, err := os.Create(settingsFilePath) + require.Nil(t, err, "could not create settings file") + err = json.NewEncoder(file).Encode(handlerSettings) + require.Nil(t, err, "could not serialize settings file") + require.Nil(t, file.Close(), "could not close settings file") + + // Write the real policy file that will be parsed in enable() + policyBytes, err := json.Marshal(policy) + require.Nil(t, err, "could not marshal policy settings") + err = os.WriteFile(filepath.Join(configFolder, constants.PolicyFileName), policyBytes, 0600) + require.Nil(t, err, "could not write policy settings file") + + return fakeEnv +} + +func hash_bytes_256(b []byte) string { + h := sha256.New() + h.Write(b) + return hex.EncodeToString(h.Sum(nil)) +} + +func make_server_with_content(content []byte) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write(content) + })) +} + +func readStatusReport(t *testing.T, env types.HandlerEnvironment, extName string, seqNum int) types.StatusReport { + t.Helper() + statusPath := filepath.Join(env.HandlerEnvironment.StatusFolder, + extName+"."+strconv.Itoa(seqNum)+constants.StatusFileExtension) + require.FileExists(t, statusPath) + + content, err := os.ReadFile(statusPath) + require.Nil(t, err) + + var report types.StatusReport + require.Nil(t, json.Unmarshal(content, &report)) + return report +} + +func Test_enable_e2e_extension_policy_settings_pass(t *testing.T) { + ctx := log.NewContext(log.NewNopLogger()) + extName, seqNum := "happyPolicyRun", 0 + scriptContent := []byte("#!/bin/bash\necho hello\n") + correctHash := hash_bytes_256(scriptContent) + + srv := make_server_with_content(scriptContent) + defer srv.Close() + + dataDir, err := os.MkdirTemp("", "policy-pass") + require.Nil(t, err) + defer os.RemoveAll(dataDir) + + policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ + LimitScripts: "alloweddownloaded", + DownloadedScriptsAllowlist: []string{correctHash}, + } + // Policy will be marshaled and written to a file in the config folder. + fakeEnv := setupPolicyE2E(t, dataDir, extName, seqNum, srv.URL+"/script.sh", false, policy) + + scriptWasExecuted := false + RunCmd = func(ctx *log.Context, dir, scriptFilePath string, cfg *handlersettings.HandlerSettings, metadata types.RCMetadata) (error, int) { + scriptWasExecuted = true + return nil, 0 + } + + err = commandProcessor.ProcessHandlerCommandWithDetails(ctx, CmdEnable, fakeEnv, extName, seqNum, constants.DownloadFolder, dataDir) + require.Nil(t, err, "enable command should succeed") + require.True(t, scriptWasExecuted, "allowed script should be executed") + + report := readStatusReport(t, fakeEnv, extName, seqNum) // verify status report exists and is valid + require.Equal(t, types.StatusSuccess, report[0].Status.Status, "status report should indicate success") +} + +func Test_enable_e2e_extension_policy_settings_block_statussuccess(t *testing.T) { + ctx := log.NewContext(log.NewNopLogger()) + extName, seqNum := "happyPolicyRun", 0 + scriptContent := []byte("#!/bin/bash\necho hello\n") + + srv := make_server_with_content(scriptContent) + defer srv.Close() + + dataDir, err := os.MkdirTemp("", "policy-pass") + require.Nil(t, err) + defer os.RemoveAll(dataDir) + + policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ + LimitScripts: "alloweddownloaded", + DownloadedScriptsAllowlist: []string{"000000000000"}, + } + // Policy will be marshaled and written to a file in the config folder. + fakeEnv := setupPolicyE2E(t, dataDir, extName, seqNum, srv.URL+"/script.sh", false, policy) + + scriptWasExecuted := false + RunCmd = func(ctx *log.Context, dir, scriptFilePath string, cfg *handlersettings.HandlerSettings, metadata types.RCMetadata) (error, int) { + scriptWasExecuted = true + return nil, 0 + } + + err = commandProcessor.ProcessHandlerCommandWithDetails(ctx, CmdEnable, fakeEnv, extName, seqNum, constants.DownloadFolder, dataDir) + require.Nil(t, err, "enable command should succeed") + require.False(t, scriptWasExecuted, "disallowed script should not be executed") + + report := readStatusReport(t, fakeEnv, extName, seqNum) // verify status report exists and is valid + require.Equal(t, types.StatusSuccess, report[0].Status.Status, "status report should indicate success") +} + +func Test_enable_e2e_extension_policy_settings_block_statusfail(t *testing.T) { + ctx := log.NewContext(log.NewNopLogger()) + extName, seqNum := "happyPolicyRun", 0 + scriptContent := []byte("#!/bin/bash\necho hello\n") + + srv := make_server_with_content(scriptContent) + defer srv.Close() + + dataDir, err := os.MkdirTemp("", "policy-pass") + require.Nil(t, err) + defer os.RemoveAll(dataDir) + + policy := &extensionpolicysettingsrc.RCv2ExtensionPolicySettings{ + LimitScripts: "alloweddownloaded", + DownloadedScriptsAllowlist: []string{"000000000000"}, + } + // Policy will be marshaled and written to a file in the config folder. + fakeEnv := setupPolicyE2E(t, dataDir, extName, seqNum, srv.URL+"/script.sh", true, policy) + + scriptWasExecuted := false + RunCmd = func(ctx *log.Context, dir, scriptFilePath string, cfg *handlersettings.HandlerSettings, metadata types.RCMetadata) (error, int) { + scriptWasExecuted = true + return nil, 0 + } + + err = commandProcessor.ProcessHandlerCommandWithDetails(ctx, CmdEnable, fakeEnv, extName, seqNum, constants.DownloadFolder, dataDir) + require.Nil(t, err, "enable command should succeed") + require.False(t, scriptWasExecuted, "disallowed script should not be executed") + + report := readStatusReport(t, fakeEnv, extName, seqNum) // verify status report exists and is valid + require.Equal(t, types.StatusError, report[0].Status.Status, "status report should indicate failure") +} diff --git a/internal/commandProcessor/commandProcessor.go b/internal/commandProcessor/commandProcessor.go index 19cab6e..25ca5e5 100644 --- a/internal/commandProcessor/commandProcessor.go +++ b/internal/commandProcessor/commandProcessor.go @@ -46,7 +46,7 @@ func ProcessImmediateHandlerCommand(cmd types.Cmd, hs handlersettings.HandlerSet } // Store handler settings locally before moving forward... - return ProcessHandlerCommandWithDetails(ctx, cmd, hEnv, extensionName, seqNum, constants.ImmediateDownloadFolder) + return ProcessHandlerCommandWithDetails(ctx, cmd, hEnv, extensionName, seqNum, constants.ImmediateDownloadFolder, constants.DataDir) } func ProcessHandlerCommand(cmd types.Cmd) error { @@ -65,10 +65,10 @@ func ProcessHandlerCommand(cmd types.Cmd) error { return errors.Wrap(err, "failed on pre steps") } - return ProcessHandlerCommandWithDetails(ctx, cmd, hEnv, extensionName, seqNum, constants.DownloadFolder) + return ProcessHandlerCommandWithDetails(ctx, cmd, hEnv, extensionName, seqNum, constants.DownloadFolder, constants.DataDir) } -func ProcessHandlerCommandWithDetails(ctx *log.Context, cmd types.Cmd, hEnv types.HandlerEnvironment, extensionName string, seqNum int, downloadFolder string) error { +func ProcessHandlerCommandWithDetails(ctx *log.Context, cmd types.Cmd, hEnv types.HandlerEnvironment, extensionName string, seqNum int, downloadFolder string, dataDir string) error { ctx.Log("message", fmt.Sprintf("processing command for extensionName: %v and seqNum: %v", extensionName, seqNum)) instView := types.RunCommandInstanceView{ ExecutionState: types.Running, @@ -80,7 +80,7 @@ func ProcessHandlerCommandWithDetails(ctx *log.Context, cmd types.Cmd, hEnv type EndTime: "", } - metadata := types.NewRCMetadata(extensionName, seqNum, downloadFolder, constants.DataDir) + metadata := types.NewRCMetadata(extensionName, seqNum, downloadFolder, dataDir) instanceview.ReportInstanceView(ctx, hEnv, metadata, types.StatusTransitioning, cmd, &instView) // execute the subcommand From f9a90209e02649138a3cc617e96653e8fb1b1e4f Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Wed, 24 Jun 2026 22:10:10 +0000 Subject: [PATCH 13/18] added e2e tests + fixed nit stuff --- internal/cmds/cmds.go | 3 ++- internal/cmds/cmds_test.go | 9 ++++++++- internal/constants/exitcodes.go | 4 +++- .../extensionpolicysettingsrc.go | 5 +++++ .../extensionpolicysettingsrc_test.go | 13 ------------- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 5f7c179..58e091b 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -243,7 +243,8 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) scriptFilePath, err := downloadScript(ctx, dir, &cfg, rceps) if err != nil && errors.Is(err, extensionerrors.ErrItemNotInAllowlist) { - return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist"), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy + os.Truncate(scriptFilePath, 0) + return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist. File has been emptied."), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy } if err != nil { errMessage := fmt.Sprintf("Failed to download script: %v due to: %v", download.GetUriForLogging(cfg.ScriptURI()), err) diff --git a/internal/cmds/cmds_test.go b/internal/cmds/cmds_test.go index e29ecc7..e7e5b00 100755 --- a/internal/cmds/cmds_test.go +++ b/internal/cmds/cmds_test.go @@ -1620,6 +1620,9 @@ func Test_enable_e2e_extension_policy_settings_pass(t *testing.T) { report := readStatusReport(t, fakeEnv, extName, seqNum) // verify status report exists and is valid require.Equal(t, types.StatusSuccess, report[0].Status.Status, "status report should indicate success") + + // Instance view is reported as the string value of "message", so it's easier to check for expected substrings. + require.True(t, strings.Contains(report[0].Status.FormattedMessage.Message, "executionState\":\"Succeeded\",\"executionMessage\":\"Execution completed"), "execution message should indicate success") } func Test_enable_e2e_extension_policy_settings_block_statussuccess(t *testing.T) { @@ -1653,8 +1656,11 @@ func Test_enable_e2e_extension_policy_settings_block_statussuccess(t *testing.T) report := readStatusReport(t, fakeEnv, extName, seqNum) // verify status report exists and is valid require.Equal(t, types.StatusSuccess, report[0].Status.Status, "status report should indicate success") + require.True(t, strings.Contains(report[0].Status.FormattedMessage.Message, "executionState\":\"Failed\",\"executionMessage\":\"Execution failed"), "execution message should indicate failure") } +// This test sets treatFailureAsDeploymentFailure to true, so failure to execute the script is reflected as a +// failed status. func Test_enable_e2e_extension_policy_settings_block_statusfail(t *testing.T) { ctx := log.NewContext(log.NewNopLogger()) extName, seqNum := "happyPolicyRun", 0 @@ -1671,7 +1677,7 @@ func Test_enable_e2e_extension_policy_settings_block_statusfail(t *testing.T) { LimitScripts: "alloweddownloaded", DownloadedScriptsAllowlist: []string{"000000000000"}, } - // Policy will be marshaled and written to a file in the config folder. + // treatFailureAsDeploymentFailure set to true fakeEnv := setupPolicyE2E(t, dataDir, extName, seqNum, srv.URL+"/script.sh", true, policy) scriptWasExecuted := false @@ -1686,4 +1692,5 @@ func Test_enable_e2e_extension_policy_settings_block_statusfail(t *testing.T) { report := readStatusReport(t, fakeEnv, extName, seqNum) // verify status report exists and is valid require.Equal(t, types.StatusError, report[0].Status.Status, "status report should indicate failure") + require.True(t, strings.Contains(report[0].Status.FormattedMessage.Message, "executionState\":\"Failed\",\"executionMessage\":\"Execution failed"), "execution message should indicate failure") } diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index f91a653..31c8972 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -10,7 +10,6 @@ const ( ExitCode_RunAsLookupUserFailed = -102 ExitCode_HandlerSettingsViolateExtensionPolicy = -103 ExitCode_DownloadedScriptBlockedByExtensionPolicy = -104 - ExitCode_ExtensionPolicyInvalid = -226 // Service Errors (-200s): ExitCode_CreateDataDirectoryFailed = -200 @@ -39,4 +38,7 @@ const ( ExitCode_ImmediateTaskFailed = -223 ExitCode_CouldNotRehydrateMrSeq = -224 ExitCode_LoadExtensionPolicySettingsFailed = -225 + ExitCode_ExtensionPolicyInvalid = -226 + + // Unknown errors (-300s): ) diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go index 8ef4890..c6a08d9 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go @@ -12,6 +12,11 @@ import ( ) func InitializeExtensionPolicySettings(ctx *log.Context, policyPath string) (*extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], *RCv2ExtensionPolicySettings, error) { + if policyPath == "" { + err := fmt.Errorf("policy path is empty") + ctx.Log("message", "policy path is empty. "+constants.ContactICMForServiceErrorsMessage, "error", err) + return nil, nil, err + } extensionPolicyManager, err := extensionpolicysettings.NewExtensionPolicySettingsManager[RCv2ExtensionPolicySettings](policyPath) if err != nil { err = errors.Wrap(err, "failed to create extension policy settings manager") diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go index c7c9e2d..d9747ee 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go @@ -131,19 +131,6 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { require.NoError(t, err) }) - t.Run("all checks pass commandId", func(t *testing.T) { - settings := makeSettings(handlersettings.CommandIdScript, "safeCommand", " Alice ", "https://example/blob") - policy := &RCv2ExtensionPolicySettings{ - LimitScripts: "allowall", - CommandIdAllowlist: []string{"safeCommand"}, - RunAsUser: "alice", - DisableOutputBlobs: true, - } - - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) - require.NoError(t, err) - }) - t.Run("all checks pass downloadedScript", func(t *testing.T) { settings := makeSettings(handlersettings.DownloadedScript, "safeCommand", " Alice ", "https://example/blob") policy := &RCv2ExtensionPolicySettings{ From 4cd140ebdf01b02d9607c59e8c2cf4f21177da15 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Thu, 25 Jun 2026 20:42:55 +0000 Subject: [PATCH 14/18] added more exit codes and updated ret values for rceps functions --- internal/cmds/cmds.go | 8 +-- internal/constants/exitcodes.go | 12 +++- .../extensionpolicysettingsrc.go | 27 ++++----- .../extensionpolicysettingsrc_test.go | 55 +++++++++++++++---- 4 files changed, 70 insertions(+), 32 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 58e091b..9396f07 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -221,9 +221,9 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma var rceps *extensionpolicysettingsrc.RCv2ExtensionPolicySettings if _, err := os.Stat(policyPath); err == nil { - extensionPolicyManagerPtr, rceps, err = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ctx, policyPath) + extensionPolicyManagerPtr, rceps, err, exitCode = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ctx, policyPath) if err != nil { - return "", "", errors.Wrap(err, "failed in enable to initialize extension policy settings"), constants.ExitCode_LoadExtensionPolicySettingsFailed + return "", "", errors.Wrap(err, "failed in enable to initialize extension policy settings"), exitCode } ctx.Log("message", "successfully initialized extension policy settings") } else if os.IsNotExist(err) { @@ -235,8 +235,8 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma // Validate handler settings against policy settings. if extensionPolicyManagerPtr != nil && rceps != nil { - if err = extensionpolicysettingsrc.ValidateHandlerSettingsAgainstPolicy(ctx, &cfg, rceps); err != nil { - return "", "", err, constants.ExitCode_HandlerSettingsViolateExtensionPolicy + if err, exitCode = extensionpolicysettingsrc.ValidateHandlerSettingsAgainstPolicy(ctx, &cfg, rceps); err != nil { + return "", "", err, exitCode } } diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index 31c8972..6ce07ca 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -8,8 +8,10 @@ const ( ExitCode_ScriptBlobDownloadFailed = -100 ExitCode_BlobCreateOrReplaceFailed = -101 ExitCode_RunAsLookupUserFailed = -102 - ExitCode_HandlerSettingsViolateExtensionPolicy = -103 - ExitCode_DownloadedScriptBlockedByExtensionPolicy = -104 + ExitCode_ScriptTypeNotAllowedByExtensionPolicy = -103 + ExitCode_CommandIdNotAllowedByExtensionPolicy = -104 + ExitCode_RunAsUserNotAllowedByExtensionPolicy = -105 + ExitCode_DownloadedScriptBlockedByExtensionPolicy = -106 // Service Errors (-200s): ExitCode_CreateDataDirectoryFailed = -200 @@ -38,7 +40,11 @@ const ( ExitCode_ImmediateTaskFailed = -223 ExitCode_CouldNotRehydrateMrSeq = -224 ExitCode_LoadExtensionPolicySettingsFailed = -225 - ExitCode_ExtensionPolicyInvalid = -226 + ExitCode_InitializeCalledWithNoPolicyPath = -226 + ExitCode_FailedToCreateExtensionPolicySettingsManager = -227 + ExitCode_FailedToGetExtensionPolicySettings = -228 + ExitCode_ExtensionPolicyInvalid = -229 + ExitCode_ValidateCalledWithNilPolicy = -230 // Unknown errors (-300s): ) diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go index c6a08d9..1c8c652 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go @@ -11,57 +11,58 @@ import ( "github.com/pkg/errors" ) -func InitializeExtensionPolicySettings(ctx *log.Context, policyPath string) (*extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], *RCv2ExtensionPolicySettings, error) { +func InitializeExtensionPolicySettings(ctx *log.Context, policyPath string) (*extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], *RCv2ExtensionPolicySettings, error, int) { if policyPath == "" { err := fmt.Errorf("policy path is empty") ctx.Log("message", "policy path is empty. "+constants.ContactICMForServiceErrorsMessage, "error", err) - return nil, nil, err + return nil, nil, err, constants.ExitCode_InitializeCalledWithNoPolicyPath } extensionPolicyManager, err := extensionpolicysettings.NewExtensionPolicySettingsManager[RCv2ExtensionPolicySettings](policyPath) if err != nil { - err = errors.Wrap(err, "failed to create extension policy settings manager") + // Manager only fails to be created if policy path is empty, so this shouldn't fail. + err = errors.Wrap(err, "failed to create extension policy settings manager. Ensure the policy path is valid") ctx.Log("message", "failed to create extension policy settings manager. "+constants.ContactICMForServiceErrorsMessage, "error", err, "policyPath", policyPath) - return nil, nil, err + return nil, nil, err, constants.ExitCode_FailedToCreateExtensionPolicySettingsManager } err = extensionPolicyManager.LoadExtensionPolicySettings() if err != nil { err = errors.Wrap(err, "failed to load extension policy settings") ctx.Log("message", "failed to load extension policy settings. "+constants.ContactICMForServiceErrorsMessage, "error", err, "policyPath", policyPath) - return nil, nil, err + return nil, nil, err, constants.ExitCode_LoadExtensionPolicySettingsFailed } rceps, err := extensionPolicyManager.GetSettings() //rceps is the pointer to the actual policy struct if err != nil { err = errors.Wrap(err, "failed to get extension policy settings after loading") ctx.Log("message", "failed to get extension policy settings. "+constants.ContactICMForServiceErrorsMessage, "error", err, "policyPath", policyPath) - return nil, nil, err + return nil, nil, err, constants.ExitCode_FailedToGetExtensionPolicySettings } - return extensionPolicyManager, rceps, nil + return extensionPolicyManager, rceps, nil, 0 } -func ValidateHandlerSettingsAgainstPolicy(ctx *log.Context, settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) error { +func ValidateHandlerSettingsAgainstPolicy(ctx *log.Context, settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) (error, int) { if policy == nil { ctx.Log("message", "no policy provided for extension policy settings") - return fmt.Errorf("no policy provided") + return fmt.Errorf("no policy provided"), constants.ExitCode_ValidateCalledWithNilPolicy } if err := ValidateScriptTypeAgainstPolicy(ctx, settings.ScriptType(), policy.LimitScripts); err != nil { - return err + return err, constants.ExitCode_ScriptTypeNotAllowedByExtensionPolicy } if settings.ScriptType() == handlersettings.CommandIdScript { if err := ValidateCommandId(ctx, settings, policy); err != nil { - return err + return err, constants.ExitCode_CommandIdNotAllowedByExtensionPolicy } } if policy.RunAsUser != "" { if err := ValidateRunAsUser(ctx, settings, policy); err != nil { - return err + return err, constants.ExitCode_RunAsUserNotAllowedByExtensionPolicy } } // TO-DO: Validate Disable Outputblob and RequireSigning once those features are implemented for RCv2. - return nil + return nil, 0 } func ValidateScriptTypeAgainstPolicy(ctx *log.Context, scriptType handlersettings.ScriptType, allowedScriptTypesString string) error { diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go index d9747ee..4dce3a4 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "testing" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/go-kit/kit/log" "github.com/stretchr/testify/require" @@ -23,10 +24,31 @@ func makeSettings(scriptType handlersettings.ScriptType, commandID string, runAs } } +func TestInitializeExtensionPolicySettings_EmptyPath_ReturnsError(t *testing.T) { + _, _, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), "") + require.Error(t, err) + require.Contains(t, err.Error(), "policy path is empty") + require.Equal(t, constants.ExitCode_InitializeCalledWithNoPolicyPath, exitCode) +} func TestInitializeExtensionPolicySettings_InvalidPath_ReturnsError(t *testing.T) { - _, _, err := InitializeExtensionPolicySettings(nopCtx(), "/definitely/not/found/policy.json") + _, _, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), "/definitely/not/found/policy.json") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to load extension policy settings") + require.Equal(t, constants.ExitCode_LoadExtensionPolicySettingsFailed, exitCode) +} + +func TestInitializeExtensionPolicySettings_InvalidPolicyFails(t *testing.T) { + tmpDir := t.TempDir() + policyPath := filepath.Join(tmpDir, "policy.json") + + payload := `{"blah blah"}` + err := os.WriteFile(policyPath, []byte(payload), 0600) + require.NoError(t, err) + + _, _, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), policyPath) require.Error(t, err) require.Contains(t, err.Error(), "failed to") + require.Equal(t, constants.ExitCode_LoadExtensionPolicySettingsFailed, exitCode) } func TestInitializeExtensionPolicySettings_ValidFile_ReturnsNil(t *testing.T) { @@ -37,11 +59,12 @@ func TestInitializeExtensionPolicySettings_ValidFile_ReturnsNil(t *testing.T) { err := os.WriteFile(policyPath, []byte("{}"), 0600) require.NoError(t, err) - _, _, err = InitializeExtensionPolicySettings(nopCtx(), policyPath) + _, _, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), policyPath) require.NoError(t, err) + require.Equal(t, 0, exitCode) } -func TestInitializeExtensionPolicySettings_CurrentBehavior_DoesNotPopulateOutputStruct(t *testing.T) { +func TestInitializeExtensionPolicySettings_PopulatesOutputStruct(t *testing.T) { tmpDir := t.TempDir() policyPath := filepath.Join(tmpDir, "policy.json") @@ -51,20 +74,22 @@ func TestInitializeExtensionPolicySettings_CurrentBehavior_DoesNotPopulateOutput out := &RCv2ExtensionPolicySettings{} - _, out, err = InitializeExtensionPolicySettings(nopCtx(), policyPath) + _, out, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), policyPath) require.NoError(t, err) + require.Equal(t, 0, exitCode) require.Equal(t, "inline", out.LimitScripts) require.Equal(t, "alice", out.RunAsUser) } // Test that validation passes and fails as expected. -func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { +func TestValidateHandlerSettingsAgainstPolicy(t *testing.T) { t.Run("nil policy", func(t *testing.T) { settings := makeSettings(handlersettings.InlineScript, "", "", "") - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, nil) + err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, nil) require.Error(t, err) require.Contains(t, err.Error(), "no policy provided") + require.Equal(t, constants.ExitCode_ValidateCalledWithNilPolicy, exitCode) }) // This test mimicks running an inline script, but policy only allows gallery scripts. @@ -75,9 +100,10 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { LimitScripts: "gallery", } - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) + err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Error(t, err) require.Contains(t, err.Error(), "script type inline is not allowed by policy") + require.Equal(t, constants.ExitCode_ScriptTypeNotAllowedByExtensionPolicy, exitCode) }) // This test mimicks running a commandId that is not in the allowlist. @@ -89,8 +115,9 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { CommandIdAllowlist: []string{"safeCommand"}, } - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) + err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Error(t, err) + require.Equal(t, constants.ExitCode_CommandIdNotAllowedByExtensionPolicy, exitCode) }) t.Run("runAs mismatch", func(t *testing.T) { @@ -100,9 +127,10 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { RunAsUser: "alice", } - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) + err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Error(t, err) require.Contains(t, err.Error(), "does not match") + require.Equal(t, constants.ExitCode_RunAsUserNotAllowedByExtensionPolicy, exitCode) }) t.Run("enforce limitScripts must be set. If not set, all commands fail", func(t *testing.T) { @@ -114,8 +142,9 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { DisableOutputBlobs: true, } - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) + err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.Contains(t, err.Error(), "script type commandId is not allowed by policy") + require.Equal(t, constants.ExitCode_ScriptTypeNotAllowedByExtensionPolicy, exitCode) }) t.Run("all checks pass commandId", func(t *testing.T) { @@ -127,8 +156,9 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { DisableOutputBlobs: true, } - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) + err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.NoError(t, err) + require.Equal(t, 0, exitCode) }) t.Run("all checks pass downloadedScript", func(t *testing.T) { @@ -140,8 +170,9 @@ func TestInitialValidateHandlerSettingsAgainstPolicy(t *testing.T) { DisableOutputBlobs: true, } - err := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) + err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, policy) require.NoError(t, err) + require.Equal(t, 0, exitCode) }) } From 620950ac652e5bc6bb274365096e1260241c9c3e Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Thu, 25 Jun 2026 20:47:28 +0000 Subject: [PATCH 15/18] added log for when script is blocked --- internal/cmds/cmds.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 9396f07..6d372a7 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -243,8 +243,12 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) scriptFilePath, err := downloadScript(ctx, dir, &cfg, rceps) if err != nil && errors.Is(err, extensionerrors.ErrItemNotInAllowlist) { - os.Truncate(scriptFilePath, 0) - return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist. File has been emptied."), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy + ctx.Log("message", "downloaded script file is not in the allowlist, attempting to truncate", "scriptFilePath", scriptFilePath) + if truncateErr := os.Truncate(scriptFilePath, 0); truncateErr != nil { + err = errors.Wrap(truncateErr, "failed to truncate downloaded script file") + return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist."), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy + } + return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist. File has been truncated."), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy } if err != nil { errMessage := fmt.Sprintf("Failed to download script: %v due to: %v", download.GetUriForLogging(cfg.ScriptURI()), err) From aef13df3916294bde0cbe26cc604421a1ee89844 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Fri, 26 Jun 2026 21:56:30 +0000 Subject: [PATCH 16/18] moved where deleting happens, edited error messages --- internal/cmds/cmds.go | 20 ++++++++++--------- .../extensionpolicysettingsrc.go | 6 +++--- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 6d372a7..cf88875 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -223,14 +223,14 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma if _, err := os.Stat(policyPath); err == nil { extensionPolicyManagerPtr, rceps, err, exitCode = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ctx, policyPath) if err != nil { - return "", "", errors.Wrap(err, "failed in enable to initialize extension policy settings"), exitCode + return "", "", errors.Wrap(err, "failed to initialize extension policy settings"), exitCode } ctx.Log("message", "successfully initialized extension policy settings") } else if os.IsNotExist(err) { ctx.Log("message", "extension policy settings file does not exist. No policy applied.", "error", err) extensionPolicyManagerPtr = nil } else { - return "", "", errors.Wrap(err, "failed to stat extension policy settings file in enable"), constants.ExitCode_LoadExtensionPolicySettingsFailed + return "", "", errors.Wrap(err, "failed to stat extension policy settings file"), constants.ExitCode_LoadExtensionPolicySettingsFailed } // Validate handler settings against policy settings. @@ -243,13 +243,9 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) scriptFilePath, err := downloadScript(ctx, dir, &cfg, rceps) if err != nil && errors.Is(err, extensionerrors.ErrItemNotInAllowlist) { - ctx.Log("message", "downloaded script file is not in the allowlist, attempting to truncate", "scriptFilePath", scriptFilePath) - if truncateErr := os.Truncate(scriptFilePath, 0); truncateErr != nil { - err = errors.Wrap(truncateErr, "failed to truncate downloaded script file") - return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist."), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy - } - return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist. File has been truncated."), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy + return "", "", errors.Wrap(err, "downloaded script file is not in the allowlist."), constants.ExitCode_DownloadedScriptBlockedByExtensionPolicy } + if err != nil { errMessage := fmt.Sprintf("Failed to download script: %v due to: %v", download.GetUriForLogging(cfg.ScriptURI()), err) extensionEvents.LogErrorEvent("enable", errMessage) @@ -913,9 +909,15 @@ func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSe ctx.Log("event", "download complete", "output", dir) if rceps != nil { - // Assume the downloaded script type is already allowed, since this was already validated earlier in enable(). + // Assume the downloaded script TYPE is already allowed, since this was already validated earlier in enable(). err = extensionpolicysettings.ValidateFileHashInAllowlist(scriptFilePath, rceps.DownloadedScriptsAllowlist, hashutils.HashTypeSHA256) if err != nil { + ctx.Log("message", "downloaded script file is not in the allowlist, attempting to delete", "scriptFilePath", scriptFilePath) + if delErr := os.Remove(scriptFilePath); delErr != nil { + ctx.Log("message", "failed to delete downloaded script file", "scriptFilePath", scriptFilePath, "error", delErr) + } else { + ctx.Log("message", "successfully deleted downloaded script file", "scriptFilePath", scriptFilePath) + } return scriptFilePath, errors.Wrapf(err, "file %s blocked by policy", scriptFilePath) } } diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go index 1c8c652..0ff0a6f 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc.go @@ -13,7 +13,7 @@ import ( func InitializeExtensionPolicySettings(ctx *log.Context, policyPath string) (*extensionpolicysettings.ExtensionPolicySettingsManager[RCv2ExtensionPolicySettings], *RCv2ExtensionPolicySettings, error, int) { if policyPath == "" { - err := fmt.Errorf("policy path is empty") + err := fmt.Errorf("policy path to initialize extension policy settings is empty") ctx.Log("message", "policy path is empty. "+constants.ContactICMForServiceErrorsMessage, "error", err) return nil, nil, err, constants.ExitCode_InitializeCalledWithNoPolicyPath } @@ -27,7 +27,7 @@ func InitializeExtensionPolicySettings(ctx *log.Context, policyPath string) (*ex err = extensionPolicyManager.LoadExtensionPolicySettings() if err != nil { - err = errors.Wrap(err, "failed to load extension policy settings") + err = errors.Wrap(err, "failed to load extension policy settings from file. Ensure the policy format is valid and the file is accessible") ctx.Log("message", "failed to load extension policy settings. "+constants.ContactICMForServiceErrorsMessage, "error", err, "policyPath", policyPath) return nil, nil, err, constants.ExitCode_LoadExtensionPolicySettingsFailed } @@ -44,7 +44,7 @@ func InitializeExtensionPolicySettings(ctx *log.Context, policyPath string) (*ex func ValidateHandlerSettingsAgainstPolicy(ctx *log.Context, settings *handlersettings.HandlerSettings, policy *RCv2ExtensionPolicySettings) (error, int) { if policy == nil { ctx.Log("message", "no policy provided for extension policy settings") - return fmt.Errorf("no policy provided"), constants.ExitCode_ValidateCalledWithNilPolicy + return fmt.Errorf("no policy provided to validate handler settings"), constants.ExitCode_ValidateCalledWithNilPolicy } if err := ValidateScriptTypeAgainstPolicy(ctx, settings.ScriptType(), policy.LimitScripts); err != nil { return err, constants.ExitCode_ScriptTypeNotAllowedByExtensionPolicy From 238c20bbc102268b1ef3ea7c61dd37e662b1eb78 Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Fri, 26 Jun 2026 22:01:45 +0000 Subject: [PATCH 17/18] fixed UTs --- .../extensionpolicysettingsrc_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go index 4dce3a4..54f938b 100644 --- a/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go +++ b/internal/extensionpolicysettingsrc/extensionpolicysettingsrc_test.go @@ -27,13 +27,13 @@ func makeSettings(scriptType handlersettings.ScriptType, commandID string, runAs func TestInitializeExtensionPolicySettings_EmptyPath_ReturnsError(t *testing.T) { _, _, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), "") require.Error(t, err) - require.Contains(t, err.Error(), "policy path is empty") + require.Contains(t, err.Error(), "policy path to initialize extension policy settings is empty") require.Equal(t, constants.ExitCode_InitializeCalledWithNoPolicyPath, exitCode) } func TestInitializeExtensionPolicySettings_InvalidPath_ReturnsError(t *testing.T) { _, _, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), "/definitely/not/found/policy.json") require.Error(t, err) - require.Contains(t, err.Error(), "failed to load extension policy settings") + require.Contains(t, err.Error(), "failed to load extension policy settings from file. Ensure the policy format is valid and the file is accessible") require.Equal(t, constants.ExitCode_LoadExtensionPolicySettingsFailed, exitCode) } @@ -47,7 +47,7 @@ func TestInitializeExtensionPolicySettings_InvalidPolicyFails(t *testing.T) { _, _, err, exitCode := InitializeExtensionPolicySettings(nopCtx(), policyPath) require.Error(t, err) - require.Contains(t, err.Error(), "failed to") + require.Contains(t, err.Error(), "failed to load extension policy settings from file. Ensure the policy format is valid and the file is accessible") require.Equal(t, constants.ExitCode_LoadExtensionPolicySettingsFailed, exitCode) } @@ -88,7 +88,7 @@ func TestValidateHandlerSettingsAgainstPolicy(t *testing.T) { settings := makeSettings(handlersettings.InlineScript, "", "", "") err, exitCode := ValidateHandlerSettingsAgainstPolicy(nopCtx(), settings, nil) require.Error(t, err) - require.Contains(t, err.Error(), "no policy provided") + require.Contains(t, err.Error(), "no policy provided to validate handler settings") require.Equal(t, constants.ExitCode_ValidateCalledWithNilPolicy, exitCode) }) From 53732f7778e31c7ada89025ab6fe14f1f301e30b Mon Sep 17 00:00:00 2001 From: alsanmsft Date: Fri, 26 Jun 2026 22:10:09 +0000 Subject: [PATCH 18/18] del unnecessary err wrap --- internal/cmds/cmds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index cf88875..43a1008 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -223,7 +223,7 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma if _, err := os.Stat(policyPath); err == nil { extensionPolicyManagerPtr, rceps, err, exitCode = extensionpolicysettingsrc.InitializeExtensionPolicySettings(ctx, policyPath) if err != nil { - return "", "", errors.Wrap(err, "failed to initialize extension policy settings"), exitCode + return "", "", err, exitCode } ctx.Log("message", "successfully initialized extension policy settings") } else if os.IsNotExist(err) {