MicrosoftPremier / VstsExtensions

Documentation and issue tracking for Microsoft Premier Services Visual Studio Team Services Extensions
MIT License
56 stars 14 forks source link

CreateWorkItem - A potentially dangerous Request.Path value was detected from the client #228

Closed jakaxd closed 6 months ago

jakaxd commented 6 months ago

Describe the context After adding the CreateWorkItem task into my yaml pipeline, I am experiencing the following error:

[error]{"$id":"1","innerException":null,"message":"A potentially dangerous Request.Path value was detected from the client (*).","typeName":"System.Web.HttpException, System.Web","typeKey":"HttpException","errorCode":0,"eventId":0}

However, if I create a seperate pipeline - the task works fine and is able to create a work item.

Expecting that the task will successfully create a work item and looking for some advice on what could be causing the issue.

Here is the task definition:

  - task: CreateWorkItem@2
    condition: | # Only create a work item if there are changes in the plan and the build is scheduled
      and(
        succeeded(),
        eq(variables['Build.Reason'], 'Schedule'),
        eq(variables['plan.changesPresent'], 'true')
      )
    displayName: 'Create Work Item'
    inputs:
      teamProject: 'REDACTED'
      workItemType: Task
      title: '$(Build.Repository.Name) - ${{upper(parameters.Environment)}} - Drift Detected'
      areaPath: 'REDACTED'
      iterationPath: 'REDACTED@currentIteration'
      fieldMappings: |
        Description=Scheduled Plan has automatically created this task due to Drift Detection in the following run: $(Build.BuildNumber).
        Priority=4
      associate: true
      # ===== Linking Inputs =====
      linkWorkItems: true
      linkType: 'System.LinkTypes.Hierarchy-Reverse'
      linkTarget: 'id'
      targetId: ${{ parameters.WorkItemId }}
      # ===== Duplicate Inputs =====
      preventDuplicates: true
      keyFields: |
        System.Title
        System.AreaPath
        System.State==[New, To Do, In Progress, Blocked, Testing]
      updateDuplicates: true
      updateRules: |
        System.History=Scheduled Plan has detected another Drift in run: $(Build.BuildNumber)
ReneSchumacher commented 6 months ago

Hi @jakaxd,

your task configuration looks good at first glance. However, the error message indicates that based on your input, the task created an API call with parameters that are deemed dangerous. This usually indicates an unescaped character or something similar. Could you please run your pipeline again with the variable System.Debug set to true and send the task log to PSGerExtSupport@microsoft.com? Please redact sensitive information in the log as needed.

The debug log should tell us which part of the task is trying to make the failing call.

jakaxd commented 6 months ago

Hi @ReneSchumacher,

thank you for the quick response! I have sent over the logs.

ReneSchumacher commented 6 months ago

Hi @jakaxd,

I found the root cause of the issue. It is a combination of a very special configuration on your side and a bug in our code. All our user inputs are pushed through a variable resolver, which replaces variables (even nested ones) with their correct values. In addition, it takes care of masking secret values in resolved variables to ensure that you cannot simply put the value of a secret variable in a work item field and, thus, make it available as clear text.

Since secret masking is just "best effort security", we use the same logic as the agent itself: a simple search of all secret values, which are then replaced by the mask ***. However, This may lead to strange behavior if, e.g., you have a variable with the regular value abc and a secret variable with the same value. In this case, even the non-secret variable value will be masked since it is the same as the secret value. Our code is even slightly worse because it also resolves non-variable values. Thus, a regular user input of abc would be masked if there is a secret variable with the value abc, even though no variable was used in the input (it simply happened to be the same value as a secret variable).

In your case, you seem to have defined a couple Terraform variables (e.g., TFVAR-DEVOPS-PROJECT-NAME) and marked them as secret. This variable definition then results in the variable resolver masking the project name, and instead of calling an API like https://dev.azure.com/yourOrg/yourProject/_apis/... our task tries to call https://dev.azure.com/yourOrg/***/_apis/....

I really wonder why nothing similar has happened before. Seems that users were lucky enough to never use conflicting secret values 😄.

As a workaround, you could make all non-secret variables (e.g., organization URI, project name, maybe even client, tenant, and subscription IDs) regular variables instead of secrets. This should allow the task to work properly. In addition, I will fix our code to ensure that we only mask secrets where needed.

jakaxd commented 6 months ago

Hi @ReneSchumacher,

Thank you for the informative response, I've done some testing by removing the secrets temporarily and this has worked!

However, I have some minor issues with this since we have a requirement that all pipeline variables must be stored in Azure Key Vault, then referenced via a variable group which will in turn make all of the variables secrets regardless of whether they are actually a 'secret' value. There are a few different ways I can now approach this, as the secrets which would be masking the URL, project name, etc aren't actually required for this pipeline so I can most likely make this work with a little re-structure of the variable groups.

Other than the Organisation URL/Project name are there any other possible secret values which are likely to have this sort of problem so we can plan ahead for the future?

ReneSchumacher commented 6 months ago

Hey,

we have discussed this internally and have decided against a change of our task. If we simply ignore secrets for specific fields like team project, area path, iteration path, work item type, etc., we might make them visible in the logs or in the work item itself. Thus, it's "safer" to break functionality if - for whatever reason - someone believes that the work item type or project must be a secret value than leaking the secret.

Oh, and to answer your question: all values, which are either used as query parameters for API calls (usually just the project and maybe the work item type) might lead to this error message. However, other fields (e.g., area path, iteration path, assigned to, or other picklist fields) might lead to other errors like "invalid field value".

jakaxd commented 6 months ago

Thanks @ReneSchumacher, we have managed to workaround the issue. Your help has been great in understanding the solution - if perhaps you do see this issue again, for us specifically, it was a combination of using the organisation url and project name as secrets. Once we removed those, it solved the problem.

I will close the issue out now, once again - thank you for the support!