It's a common approach, but I wouldn't call it the best approach for writing modules because it makes the module import times slower. It's better to have a build script that takes all of your separate script files and merge them into one big psm1 file.
You sort of need a build process anyway to keep the module manifest up to date so might as well merge the files in that process anyway. Here's a module built specifically for this: https://github.com/PoshCode/ModuleBuilder

GitHub

GitHub - PoshCode/ModuleBuilder: A PowerShell Module to help scripters write, version, sign, package, and publish.

A PowerShell Module to help scripters write, version, sign, package, and publish. - GitHub - PoshCode/ModuleBuilder: A PowerShell Module to help scripters write, version, sign, package, and publish.

I load (source) all of functions from $profile:

. /path/to/functions.ps1

@MartinGC94, nice module but just trying to understand what it exactly does as it might indeed improve the performance by merging all the files but doesn't resolve the "freeform input"/scope issue that trying to hilight here.

In fact, I see the same questionable approach of blindly including files in the shared module scope in: ModuleBuilder/Source/ModuleBuilder.psm1

But let me ask: what would happen if I put Set-StrictMode Latest in the root of one of the .ps1 PowerShell files? Will it be included in the merged .psm1 file?

@237dmitry,

I load (source) all of functions from $profile:

Please explain how this approach will be safer as from my view, using the profile might even bring over a larger bad practice/(unintended) injection issue, see: #17148 Profiles shouldn't unnoticeable shared through UAC

nice module but just trying to understand what it exactly does as it might indeed improve the performance by merging all the files but doesn't resolve the "freeform input"/scope issue that trying to reveal here.

I haven't actually used it, I just stick to my normal build scripts but I've read the ReadMe and can see it does the same as my scripts so it's just a question of how they are doing it. If they are using the parser to extract the function definitions then it shouldn't include other unrelated statements like Set-StrictMode that are set outside the function definitions. If they are taking the whole file then such statements will pollute the whole module scope.
The psm1 file is really just a fancy script file that is run when the module is imported, this is necessary so C# type definitions added with Add-Type can be used, config files can be read and module scoped variables can be set, etc.
If you can't trust the content of the .ps1 files you use to build the module then you need to have a review process before you build and publish new modules.

For strict mode specifically, it is set in the scope so if you want to set it in one function and not the whole module you can simply set it inside the function like this:

function MyFunction
{
    Set-StrictMode -Version 3.0
    #This throws an error
    $ThisVarDoesNotExist
}
function MyFunction2
{
    #This doesn't throws an error
    $ThisVarDoesNotExist
}

@MartinGC94, I understand that.
Just to be clear, I am trying create a common sense here and prevent pitfalls for others by defining a best practice (similar to avoiding Invoke-Expression) where the approach is primarily about preventing unintended execution of code (code injection) by not (easily) allowing it.

    $Value = Get-Content -Raw $Function
    New-Item -Path function: -Name $Name -Value $Value -Force

FYI this would disable breakpoints from working correctly. I ask that you don't propagate things like this primarily because folks end up copying it blindly and then they think VSCode is broken when debugging doesn't work anymore. If you use a pattern like this in your own projects, adding a comment saying that it breaks debugging would be very appreciated.

That said, this might be a better discussion for https://github.com/PoshCode/PowerShellPracticeAndStyle

GitHub

GitHub - PoshCode/PowerShellPracticeAndStyle: The Unofficial PowerShell Best Practices and Style Guide

The Unofficial PowerShell Best Practices and Style Guide - GitHub - PoshCode/PowerShellPracticeAndStyle: The Unofficial PowerShell Best Practices and Style Guide

@SeeminglyScience,

If you use a pattern like this in your own projects...

Thanks for the feedback, I haven't implement anything like this yet, it was just a flawed possible direction which I would like to hold against you (and I am happy I did).
Yet, I think the actual issue stays: with the general accepted approach, all the imported scripts share the the same (module) scope where actually only a function should be imported.

with the general accepted approach, all the imported scripts share the the same (module) scope where actually only the function should be imported.

There are a lot of other things that can be (and commonly are) included in a functions script file like script scope variable declarations, Register-ArgumentCompleter and Export-ModuleMember.

I can definitely understand the intent and I think you bring up some really interesting concepts that I don't want to discourage exploration of. I don't think it buys anything from a security perspective though.

@SeeminglyScience,

I don't think it buys anything from a security perspective though

I am not aiming for a security that (fully) prevents code injection (of malicious software) but protecting a scripter from unintended execution of code that might overwrite items as general variables and settings (similar to avoiding Invoke-Expression)

Export-ModuleMember

This exactly an example that I would like to prevent (correct me if I am wrong, but I like to state): Don't even think about putting this in the dot-sourced (included) .ps1 PowerShell file, it is a responsibility of the main .psm1 module file.

Register-ArgumentCompleter

Yes, this is an issue in the approach I would like to see. The same limitation exist for installing (Install-Script) from the PowerShell Gallery, see also the Set-Alias examples in: #13883 Enhance standalone script installation

Export-ModuleMember

This exactly an example that I would like to prevent (correct me if I am wrong, but I like to state): Don't even think about putting this in the dot-sourced (included) .ps1 PowerShell file, it is a responsibility of the main .psm1 module file.

I don't really see a reason to draw this line, dot sourcing is just invoking a script file as if it's in the same scope. I don't see an issue with also including tasks related to the function that aren't strictly the function definition

Register-ArgumentCompleter

Yes, this is an issue in the approach I would like to see. The same limitation exist for installing (Install-Script) from the PowerShell Gallery, see also the Set-Alias examples in: #13883 Enhance standalone script installation

Sorry I mean Register-ArgumentCompleter targeting the function that is also in that file.

@SeeminglyScience,

I don't really see a reason to draw this line...

I think there is a misunderstanding at either site.
In the use case how it is setup in our environment which is apparently a common way of Creating powershell modules from multiple files, referencing with module. This means that aside from .psm1 module file there is a Functions folder that contains several "function script" (.ps1) files. Each "function script" file (e.g. Send-Greeting.ps1) contains just a single function (e.g. function Send-Greeting { ... }). Each function is "imported" in the module using dot-sourcing:

foreach ($Function in $Functions) { . $Function.Fullname }

In our organization, the "function scripts" are maintained by multiple developers.
The issue is that I actually do not want to Invoke (dot-source) the concerned "function scripts" but rather Import the actual functions because the pitfall with invoking the "function script" is that if someone defines something outside the actual function (as e.g. Set-StrictMode) might break another function. Or in case of Export-ModuleMember might impact the whole module (as -afaik- Export-ModuleMember is designed to be invoked only once per module).

I our organization, the "function scripts" are maintained by multiple developers. The issue is that I actually do not want to Invoke (dot-source) the concerned "function scripts" but rather Import the actual functions because the pitfall with invoking the "function script" is that if someone defines something outside the actual function (as e.g. Set-StrictMode) might break another function.

That's a fair reason for an organizational rule or personal preference. Would be nice to have a non-default PSScriptAnalyzer rule to allow folks to enforce that at the CI/CD level if desired.

I understand the intent a bit more now, thank you for explaining. I still would not recommend it to everyone as a general best practice, but I can definitely see the value in the option.

Or in case of Export-ModuleMember might impact the whole module
(as -afaik- Export-ModuleMember is designed to be invoked only once per module).

Nah it can be invoked any amount of times. It doesn't exactly play well with command discovery static analysis but that doesn't matter as long as you have a manifest (and wildcards have the same issue anyway which rules out most uses).

For what it's worth, I agree with @SeeminglyScience here, this is a "best practices" question not really addressable here.

@Jaykul,

For what it's worth, I agree with @SeeminglyScience here, this is a "best practices" question not really addressable here.

I have added your suggestion from PoshCodeModule#157 Module structure issue in here:

For what it's worth, @iRon7 if you don't merge all the files, you miss out on 1-3 above, but if you also don't dot-source, you loose ModuleScope, along with the ability to share private functions -- which are powerful and useful features, and the main reasons to build modules in the first place. Honestly, if you want to give those up, you're probably not writing a module.

If you need to ship scripts, you can publish individual scripts.

If you need to ship a collection of scripts, you could put them all in the module folder, and have a one-liner .psm1:

$Env:Path += ";$PSScriptRoot"

Or export an alias that points at the file (Invoke-Build does that).

This way, the scripts are each fully independent, and can't break each other (at least, not accidentally).

You could also Import-Module each ps1 file, or shove them in NestedModules in your psd1. Either way, they're stand-alone and don't get in each other's scope (at least, not without intent), and you're more likely to list them all out and prevent someone dropping an extra in without anyone noticing.

I don't think that adding the scripts to the environment path is feasible and a "better praktisch" than dot-sourcing "function scripts". Taking our companies environment where we have about 25 repositories/modules (each with 3 till around 20 "function scripts").

Your, idea about "NestedModules in your psd1" could have some potential as a better praktisch than dot-sourcing "function scripts" but I can't see this completely through yet with respect to advantages and disadvantages.

Anyways (our environment is already life for years and it unlikely going to change based on any better practices), what I am trying to achieve with this issue is a solid definition of how exactly functions should be imported into a module before moving this to a document-issue. Personally I feel that PowerShell lacks a better praktisch for this and might even require something like a Import-Function command.

As with the Avoid using Invoke-Expression document, I think that a similar document like e.g. "Avoid dot-sourcing function scripts into a module" should exist. As you point out yourself: "Dot-sourcing has minor security risks" where I think that the risks are actually slightly higher compared to using Invoke-Expression , as it is more likely that multiple engineers work on the same module.

But the thing is that if state to avoid something, you should be able to have solid definition of what you should do instead...

As you point out yourself: "Dot-sourcing has minor security risks" where I think that the risks are actually slightly higher compared to using Invoke-Expression , as it is more likely that multiple engineers work on the same module.

The only real security risk is that someone could add a file that then gets dot sourced (which you can solve by using a fixed list of files to dot source). But if you aren't locking down the environment with CLM and allow listing via signing, that same risk is present in any of these forms. An attacker can modify the psm1 file itself, they can modify a function definition which would only delay the execution of malicious code, etc.

The reason Invoke-Expression is discouraged is that it's often used to invoke arbitrary code from user input. This gives a super easy way to bypass CLM if done from an allow listed script/module.

Having multiple developers contributing to a module is not a security risk. Trying to build a module from unrelated functions written by trusted developers, without actually putting effort into making those functions work together in a module is likely to cause problems though.

I think the best solution for unrelated scripts is to put them in your path, and not in a module. If they're related, then the best practice to avoid regression bugs is to write tests, and run them all before you ship the module.

In my opinion, the best practice for shipping script modules is to ship a single PSM1 file with all the classes and functions in it.

For what it's worth, my suggestion for isolating using NestedModules wasn't sufficiently tested...

If you leave your source files as .ps1 scripts, then NestedModules (or Import-Module in the psm1) acts the same as dot-sourcing. In fact, you can see in the -Verbose output actually says it's dot-sourced, and the variables spill over into other functions:

If you change the extensions to psm1, then each one is isolated, and the variables don't cross over (nor does the StrictMode):

I think the new microsoft-github-policy-service should skip any issues that have referenced in the last 6 months ("This was referenced on Oct 4").

Besides, this issue Needs-Triage and either marked with something like Resolution-Declined or put it on a kind of wishlist "Issue we would like to prioritize, but we can't commit we will get to it yet"