Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Allow command execution, for everything not supported out of the box #5

Closed
gaelcolas opened this issue May 17, 2016 · 14 comments
Closed

Comments

@gaelcolas
Copy link
Contributor

gaelcolas commented May 17, 2016

First of all, great idea!

I believe there are things that you won't be able to provide a (safe) plaster interface for, and there will always be some repetitive execution that we'll want to automate.
It's good to not have execution embedded in manifest files (XML, JSON, PSD1) but IMO it should still provide the flexibility to 'run arbitrary code', just not from the manifest, from a ps1 file, to make reviewing easier.
One way to implement this could be that the manifest only provides 'reference' to psake tasks. So if you want to run any arbitrary task, it's in a psake file, making reviewing the module easier and quicker.

One example is that I often add one of my repo, as a git submodule on my projects, so that it lives under moduleName/lib/subLibName.
To do so, I want to have an option when I create a new module (similar to what Plaster offers), but that option will do something like:

cd ./lib/
git submodule add [email protected]:Project.git

I don't think Plaster could support 'safe' templating with those functionality for all eventuality (git submodule, other SCM, Creating Github/bitbucket repo through API/Module, opening JIRA project...), so what are we trying to keep 'safe'?

I understand that you'd want the templates to be as safe as possible to avoid malicious injection, but how's that different from the modules in the PowerShell Gallery? Installing someone else's module and using it is not safe. Using 'yet another nuget repository' will not help, and although it might be nice to have separation of 'duty', I don't think it's very scalable (you won't have a different nuget repo for every type of packaging, or you'd have a different one for DSC resources, and that would require a different security model?), and the security of a Module template should probably be treated the same as the one from a Module (which can be malicious as well).

The creation of a specialist package for this sounds like a nice idea, but I'd probably wrap it inside a PowerShell module. What's the gallery need would be more METADATA to support this new 'Type'.

The alternative to this, obviously, is that you keep it safe, and someone (or many people, independently) will leverage it and wrap around to add the 'arbitrary execution'.

Hope that's clear, feel free to ask for clarifications.

@daviwil
Copy link
Contributor

daviwil commented May 19, 2016

Gael put together a change which adds an execute task: master...gaelcolas:CmdExecution

I can see some cases where this might be useful. Security is definitely a concern, but could we alert the user to the fact that the template is going to be executing and prompt to ensure the user is OK with this?

@rkeithhill
Copy link
Collaborator

Implementation of this is pretty trivial. Whether we want to put this in at all or for v1 - that's the question to explore. :-) And it is certainly worth exploring. I have my opinions but I'd love to hear from more of the PowerShell community - especially Kirk and Jaykul- if we can rope them in to review this. For now, here are some of the concerns I have.

One issue I mentioned to Gael this morning was idempotency. Right now you can run the template multiple times on the same directory and it will check files and if they're identical, it will leave them untouched. But if they are different then you get a conflict prompt to overwrite or not.

Some arbitrary code may be safe to execute multiple times and other code may not. Not sure what happens if you run git init on an already initialized repo. But you can easily imagine script that isn't meant to be run more than once. IIRC this is also an issue with NuGet's use of install.ps1 and uninstall.ps1. They've deprecated those scripts for v3. Although I think they do still support an init.ps1 script. Hmm...

I guess we could just prompt each time. We could also add an attribute to execute that allows the template author to declare that they know the command is not safe to run multiple times. It would be a poor user experience to require the user to know this. That said, I'm not entirely sure how to check if the template has run before to even know to skip such a command on subsequent invocations.

Heh, this reminds me a bit of the InstallShield script to MSI conversion. With IS script, anything went and a clean uninstall was hit or miss. MSI came along - all declarative / table driven - and cleaned that up. Uninstalls worked pretty well. Until folks wanted to run arbitrary code during install (hello custom actions). And now we're back poor uninstalls. Maybe the new "modern installer" (Appx / DesktopAppConverter) will clean this back up - again. :-)

I don't think it is a core feature for the initial release but that doesn't mean I can't be convinced otherwise. I just want to make sure we have more than a few data points before we make a decision.

BTW I did start an issue on capturing & debating use cases. Still not sure that automatically initializing a Git repo is a core user scenario.

@gaelcolas
Copy link
Contributor Author

It's true that implementing it was easy, the benefit now is that we can experiment (without polluting the master), and see what needs modifying and I'll get to that latter.
I've slept on it, and I agree with Keith, that executing task is probably not the core scenario you are trying to (and should) address, at least in v1. It would be much easier to add it later down the line, than remove it because it confuses people or is abused.
At the same time, I see sort of two conflicting approach, probably historic between the Dev and the Ops:

  • Dev mindset wants surgical tools that do very precise tasks, very well (rigour)
  • Ops mindset (like me) wants hammers because everything looks like nails

If one of the two should change, it's probably the Ops, but we do this for reason, so we still have the need.
So that you can focus on your vision, and I can implement the 'extra' feature I need (and some other might do as well) without forking apart, it would be great to agree on a pattern to extend your work, independently.
I'm thinking for instance of an 'UnsafePlaster' module that would use the 'Plaster' module as a nested module, but would add the execute feature as per the proto, but proxy/expose the base functions.
I know your code will evolve and there will be breaking changes, but if the extensibility use case is thought from now then at least we can catch breaking changes and and fix them early.

Does that seem possible/feasible and more reasonable?

@daviwil
Copy link
Contributor

daviwil commented May 20, 2016

I'm going to think of some ways that we might be able to enable some form of code execution task while maintaining the design principles that Keith is using. I'd prefer to not introduce extensions just to enable code execution because it may complicate the user experience. I definitely don't rule out the idea of extensions but I think we should be sure on what things we want to include in the core module first.

I want to get more input from folks in the community and also from people in the team. @KirkMunro, @Jaykul, @dlwyatt: what do you think about project templates having the ability to execute arbitrary code? Safe or unsafe? Should we include that as a feature?

Keith, anyone else we should bring into the discussion?

@rkeithhill
Copy link
Collaborator

Maybe we pull in @oising as well.

@dlwyatt
Copy link
Member

dlwyatt commented May 20, 2016

I agree that there's not really any more risk of malicious code than using any other shared module or script. However, perhaps you should treat them like macros in Office documents: big warnings before you actually execute the code, unless it's signed by a trusted publisher.

@oising
Copy link

oising commented May 20, 2016

Whut, who's calling me? I hadn't heard of Plaster before now - but Yeoman for PowerShell makes hella sense to me. So, embedded scripts? Without giving much more than ten seconds of thought, if it's intended to be used like TT/T4, then constrained language mode might be a thought?

@rkeithhill
Copy link
Collaborator

rkeithhill commented May 20, 2016

There are some aspects of T4 in the sense that a file can be copied as-is from template to destination. Or it can be copied and expanded e.g.:

MIT License

Copyright (c) <%=$PLASTER_YEAR%> <%=$PLASTER_PARAM_FullName%>

Permission is hereby granted, free of charge, to any person obtaining a copy
...

Restricted execution ie "safe" templates was an original desire. If I scaffold using this template the only thing it can do is create files/folders (or modify existing files) in the destination path. But folks are already asking for features such as being able to initialize a Git repo or generate script based on some external metadata.

We could add begin/end hooks for running arbitrary scripts and then warn the user about this - asking them if they trust the template (or template author). If your template doesn't use these arbitrary script hooks then the user won't see such warnings.

I'm still a little tentative on this since the thought is that you could run the same template twice on a destination. Say you muffed up the manifest or something and want to recreate it. Running the template twice right now is safe. Just like with Yeoman, we can easily make copying/modifying files safe by detecting conflicts and warning the user about overwriting. Arbitrary script - well - we either push the "execute it" decision to the user or the template author (or both) and hope they know what they're doing. :-)

@Jaykul
Copy link

Jaykul commented May 20, 2016

The right answer here is "what you're trying to do has nothing to do with Plaster"
Like this.

Plaster should focus on helpers to scaffold applications and making generators more easily reusable...

If users want to run something at the beginning or the end, they can just paste that in a .ps1 file and run that!

Frankly, if Plaster goes anywhere near psake, it's going in the wrong direction, and at the wrong end of my development process.

@larssb
Copy link

larssb commented Jun 22, 2017

Did @Jaykul "kill" the discussion with the link to the stackoverflow yeoman question?

I agree that wanting to execute code or minor tasks is not a core part of Plaster. However, I would like to help the end-users of the Plaster template I'm doing, as much as possible. They are not experienced PowerShell coders. Therefore it would be great to have as much as possible bootstrapped for them. In that way they can get to the PowerShell coding that will make an actual difference for them/the task they are trying to solve.
Okay I could have Plaster give a message that they should run this or that afterwards, but I would just be great to have it done for them already.

Any thoughts and feel free to ask if anything needs clarification.

Thank you.

@rkeithhill
Copy link
Collaborator

At this point, I'd rather entertain adding new directives to perform common actions. One example is a directive to initialize a local Git repo. For arbitrary script - at this point I recommend exactly what you suggest - putting that stuff in a script file and use message to request the user run the script file to "finish" workspace prep.

@larssb
Copy link

larssb commented Jun 23, 2017

Hi @rkeithhill,

New directives would certainly also do it. Would be great. As it is very likely going to specific scenarios users would like covered by Plaster templates, like:

  • git repo initialization
  • API call to a documentation site to get the latest and greatest on how x company wants things to be done in regards to developing software for them.
  • Checking for tooling pre-requisites. For example if PowerShell is part of a project using NodeJS, TypeScript or something similar.

Should I do a feature request on such directives. As of now, git repo initialization is the most wanted, or is this issue enough?

Thank you very much.

@rkeithhill
Copy link
Collaborator

Would you mind creating an issue for each? This would allow me to close each issue as it gets implemented and they're not all likely to get implemented at once. Don't need much detail for the git init scenario but add just a bit more "use case" info to the other two. Thanks!

@larssb
Copy link

larssb commented Jun 23, 2017 via email

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

8 participants