Azure / arm-ttk

Azure Resource Manager Template Toolkit
https://aka.ms/arm-ttk
MIT License
441 stars 188 forks source link

Incorrect Regex (Length Check) Validation #731

Closed dvdmtw98 closed 1 year ago

dvdmtw98 commented 1 year ago

Description

When using arm-ttk to validate createUiDefination.json file the regex validation check is flagging regex's that have length's specified as not having length condition.

Validation Result

Regex Validation

UI Element Definition

{
    "name": "objectId",
    "type": "Microsoft.Common.TextBox",
    "label": "Object Id",
    "toolTip": "Id of User or Service Principal that can access secrets from Key Vault",
    "constraints": {
        "required": true,
        "validations": [
            {
                "regex": "^[0-9a-fA-F]{8}-(?:[0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}$",
                "message": "Invalid Object Id Pattern"
            }
        ]
    },
    "visible": true
},
{
    "name": "name",
    "type": "Microsoft.Common.TextBox",
    "label": "Data Factory Name",
    "toolTip": "Only supported characters can be used in name",
    "constraints": {
        "required": true,
        "validations": [
            {
                "regex": "^(?!-)[A-Za-z0-9-]{3,63}(?<!-)$",
                "message": "Only alphanumeric characters are allowed, and value must be 3-36 characters long"
            }
        ]
    },
    "visible": true
}

Regex's being incorrectly flagged

^[0-9a-fA-F]{8}-(?:[0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}$
^(?!-)[A-Za-z0-9-]{3,63}(?<!-)$

Observations

For the 2nd regex on removing the negative lookbehind the validation passes. Possibly the current check only looks at the end of the regex to see if a length has been provided. As for the 1st regex event though it ends with a length it is still flagged as a warning.

StartAutomating commented 1 year ago

@dvdmtw98 thanks for bringing this up.

I believe your triage to be correct and your set of exceptions to the rule to be appropritate.

If you'd like to take a crack at correcting this, let me know (and let me know if there's any guidance you'd like).

dvdmtw98 commented 1 year ago

I have never worked with PS scripts but sure I can give it a try.

Had a quick look at the files and the tests for the length check seems to be in CreateUIDefinition/Textboxes-Are-Well-Formed.test.ps1 file.

Is there any reason why two separate regex to check the start and end of the textbox value is used ? Won't it be sufficient to only check if the length condition {N, M} occurs somewhere in the string ?

abiswas25 commented 1 year ago

I too have a similar issue. Below is a snippet showing a textbox field in createUiDefinition.json which accepts an IP address:

{
    "name": "NetworkInterfacePrivateIP",
    "type": "Microsoft.Common.TextBox",
    "label": "Network Interface Private IP Address",
    "toolTip": "Network Interface Private IP Address",
    "constraints": {
        "required": true,
        "regex": "^(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})$",
        "validationMessage": "Enter a valid IP address."
    },
    "visible": true
}

Running the toolkit test I get warnings as below:

[?] Textboxes Are Well Formed (219 ms)
    TextBox 'NetworkInterfacePrivateIP' regex does not have a length constraint.
    TextBox 'gateway' regex does not have a length constraint.
    TextBox 'subNetmask' regex does not have a length constraint.
    TextBox 'primaryDNS' regex does not have a length constraint.
    TextBox 'secondaryDNS' regex does not have a length constraint.

Since we are already limiting the number of digits which can appear for each part of the IP address, there is no need to have an explicit constraint for total length. Is my understanding correct that these warnings can be treated as a false flag?