pnp/cli-microsoft365

Validation fails to detect empty string for "id" Parameter in "m365 spo list get" command.

Opened this issue · 2 comments

Saurabh7019 commented

Priority

(Low) Something is a little off

Description

The validation for the id parameter in the m365 spo list get command does not properly handle an empty string. This results in the validation passing incorrectly and returning same result as m365 spo web get

Similar issues exists in a few other commands within the CLI.

Steps to reproduce

  • Run the m365 spo list get --id "" --webUrl https://contoso.sharepoint.com command.
  • Notice that the validation does not fail as expected and the command is executed despite the empty id.

Expected results

The validation for the id parameter should properly handle an empty string.

Actual results

The command executes without proper validation and returns spo web get result.

Diagnostics

No response

CLI for Microsoft 365 version

v7.9.0

nodejs version

v20.9.0

Operating system (environment)

Windows

Shell

PowerShell

cli doctor

No response

Additional Info

image

The issue is with unhandled empty string in command validation.

milanholemans commented

Yes, unfortunately, this is a "known" issue. Currently, we mostly check on if (args.options.id) but in fact, we should if (args.options.id !== undefined) because an empty string validates to false. Unfortunately, the majority of all commands suffer from this.
@waldekmastykarz I'm wondering if this is solved within ZOD? If that's the case, I don't think it's worth fixing it.

waldekmastykarz commented

Yes, unfortunately, this is a "known" issue. Currently, we mostly check on if (args.options.id) but in fact, we should if (args.options.id !== undefined) because an empty string validates to false. Unfortunately, the majority of all commands suffer from this. @waldekmastykarz I'm wondering if this is solved within ZOD? If that's the case, I don't think it's worth fixing it.

Yes, this should be fixed in zod, because it has a separate required/optional check. I suggest, that we prioritize fixing this issue by migrating this command to zod as soon as possible so that we can confirm the fix.

  • 👍1