Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor error message on some rule names #1012

Closed
nightroman opened this issue Mar 12, 2022 · 6 comments · Fixed by #1013
Closed

Poor error message on some rule names #1012

nightroman opened this issue Mar 12, 2022 · 6 comments · Fixed by #1013
Assignees
Labels
breaking-change Changes that affect existing functionality bug Something isn't working enhancement New feature or request
Milestone

Comments

@nightroman
Copy link

Poor error message on some rule names

If a rule name contains some characters (not supported? documented?), for example ",
then PSRule commands fail with cryptic messages "Illegal characters in path."
with no other details.

Steps to reproduce and actual results

(1) Create the rule script My.Rule.ps1

Rule 'My rule "1"' {
    $true
}

(2) Invoke

Get-PSRule .\My.Rule.ps1

Result:

Get-PSRule : Illegal characters in path.
At line:1 char:1
+ Get-PSRule .\My.Rule.ps1
+ ~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [Get-PSRule], ArgumentException
    + FullyQualifiedErrorId : System.ArgumentException,Get-PSRule

(3) Invoke

1 | Assert-PSRule -Path .\My.Rule.ps1

Result:

[ERROR] Illegal characters in path.

Expected results

PSRule commands should either work or fail with clear error messages explaining which rule has not supported name.

Version

2.0.0 Prerelease = 'B2203033'

@BernieWhite BernieWhite added the bug Something isn't working label Mar 12, 2022
@BernieWhite
Copy link
Member

@nightroman Thanks for reporting this issue.

@BernieWhite BernieWhite self-assigned this Mar 13, 2022
@BernieWhite BernieWhite added this to the v2.0.0 milestone Mar 13, 2022
@BernieWhite
Copy link
Member

BernieWhite commented Mar 13, 2022

Interestingly this is due to path validation further down the stack with Windows PowerShell 5.1 looking for help files with the same name as the rule. The issue is not present in PowerShell 7.2.

Regardless it's overdue for some validation and better guidance on rule/ resource names.

Resource names should meet the following regex from PSRule v2.

^[a-zA-Z0-9][a-zA-Z0-9._-]{1,126}[a-zA-Z0-9]$

@BernieWhite BernieWhite added breaking-change Changes that affect existing functionality enhancement New feature or request labels Mar 13, 2022
BernieWhite added a commit to BernieWhite/PSRule that referenced this issue Mar 13, 2022
BernieWhite added a commit to BernieWhite/PSRule that referenced this issue Mar 13, 2022
BernieWhite added a commit to BernieWhite/PSRule that referenced this issue Mar 13, 2022
@nightroman
Copy link
Author

nightroman commented Mar 13, 2022

@BernieWhite

^[a-zA-Z0-9][a-zA-Z0-9._-]{1,126}[a-zA-Z0-9]$

I think it's a little harsh, if not plain "wrong". What about names in not just English? Or human friendly well readable short sentences, say, with spaces?

@BernieWhite
Copy link
Member

@nightroman Thanks for the feedback. It warrants further thought.

@BernieWhite
Copy link
Member

@nightroman It's not a clear as I would like, however the following regex should leave naming open without clashing with PSRule or OS specific invalid characters.

^[^<>:/\\|?*\"'`+@._\-\x00-\x1F][^<>:/\\|?*\"'`+@\x00-\x1F]{1,126}[^<>:/\\|?*\"'`+@._\-\x00-\x1F]$

Thoughts?

@nightroman
Copy link
Author

Thoughts are the more names allowed the better. But you should be reasonably strict to satisfy the current and potential future PSRule requirements. As for the regex, it looks reasonable but I cannot tell much without knowing exactly what the requirements are.

BernieWhite added a commit that referenced this issue Mar 15, 2022
* Fixes unclear error on invalid rule name #1012

* Integrated further community feedback

* Update docs

* Fix minor typo

Co-authored-by: ArmaanMcleod <armaan_mcleod@outlook.com>
@BernieWhite BernieWhite mentioned this issue Mar 25, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes that affect existing functionality bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants