Typo. Should be - Roslyn analyzers

Thanks!

Fixed.

Maybe tweak to The following property ...

Fixed.

Use of multiline comment seems a bit out-of-place. Consider using //

Using // raise CodeFactor issue (blank line after the comment).

I think CodeFactor recommends using //// to comment out code specifically, which bypasses the usual warning for that.

@xtqqczze Have you thoughts?

It seems that for consistency we should use // and ignore the codefactor issue

Tagging for @wg-security review @PaulHigin

Notice, the file is written to disk and it is accessible for compliance tools.

@PaulHigin Friendly ping.

@TravisEz13 , @iSazonov
The concern, of course, is code injection. I don't immediately see an issue, but this needs to be discussed in the next WG-Security meeting. However, a meeting may not occur until December, due to the holidays. Since this is an optional change, I feel it is not high priority.

@WG-Security We looked into this and our main concern is that this is a potential code injection vulnerability. The version properties are all passed in as string types, and there is no validation. In addition the properties are passed in a very non-transparent manner, and unwanted injection would be difficult to detect.

We feel using something like a codedom to generate the source rather than string concatenation, is desirable.

@PaulHigin Could you please clarify what code you have reviewed - the source generator itself or resulting code generated by the source generator?
The SG is only used at compile time (not at runtime) and it is not distributed to end PowerShell users. I don't think it can be a security problem like you said.
The generated code contains only static strings and the file (.\src\System.Management.Automation\gen\SourceGeneratedFiles\PSVersionInfoGenerator\System.Management.Automation.Internal.Generators.PSVersionInfoGenerator\PSVersionInfo.generated.cs) is:

// <auto-generated>
// This file is auto-generated by PSVersionInfoGenerator.
// </auto-generated>

    namespace System.Management.Automation
{
    public static partial class PSVersionInfo
    {
        // Defined in PowerShell.Common.props as ProductVersion
        // '6.0.0-beta.7 Commits: 29 SHA: 52c6b...'
        internal static string ProductVersion { get; } = "7.2.0-preview.10 Commits: 169 SHA: 795a1ca556caa9cf90e43e63b9696b2437dc722e";

        // Defined in PowerShell.Common.props as
        // git describe --abbrev=60 --long
        // 'v6.0.0-beta.7-29-g52c6b...'
        private static readonly string _rawGitCommitId = "7.2.0-preview.10-169-g795a1ca556caa9cf90e43e63b9696b2437dc722e";

        // Defined in PowerShell.Common.props as PSCoreBuildVersion
        // '6.0.0-beta.7'
        private static readonly string _mainVersion = "7.2.0-preview.10";
    }
}

To be clear, this is the opinion of the @WG-Security group, not just me. We looked at const string SourceTemplate where the above property values are concatenated into the source template as strings. This is a supply chain source code injection opportunity, that would be difficult to detect. Our understanding is that using a CodeDom is the preferred way to build up source code for compilation.

@PaulHigin I really don't understand about CodeDom. The best comment from .Net team I saw about using CodeDom in SGs is dotnet/roslyn#48214 (comment)

I'm not an expert either, and I'll defer to @TravisEz13.

For code injection to happen here, there would need to be a double quote, right? Can we just reject any strings that contain a double quote?

I really don't understand. We could implement this as TypeGen and ResGen which we already use. Only difference is the TypeGen and ResGen are our custom utilities but the source generator is .Net feature.
To do injection, someone would have to compromise the entire repository in order to corrupt both build scripts and the source code.