-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Kagre preserve working path on impersonate #3
base: main
Are you sure you want to change the base?
Kagre preserve working path on impersonate #3
Conversation
preserve the current working path if possible, otherwise fall back to the root on the system drive
Force the current path to be the root on the system drive before switching contexts
restore working path prior to Push-ImpersonationContext
forgot this one on the last commit
name the path stack so that it doesn't interfere with the user's pushd stack
name the path stack so that it doesn't interfere with the user's pushd stack
functional --> fanciful: match the project's coding style
functional --> fanciful, update to match the project's coding style
.INPUTS is for the pipe-stream, not the parameters
somehow in my edits, had gotten the order of these reversed
apparently not all non-interactive contexts are created equal... The context in which the appveyor auto-build is running should be non-interactive; however the old code failed to detect this; so as patch I've added a way for the caller to explicitly state that Use-VaultSecureString should have been invoked as non-interactive.
the build fails, this is hopefully a workaround; however the build should probably not use the project it's building
appveyor build updates
This branch may now try to fix too much. I was hoping to eliminate the build failure, but now the issue is probably that the version imported is older than the one being built... still it's not a compiler probably shouldn't be using it to build itself. |
@Kagre |
Sorry, I'm still not super familiar with git/hub... where might I find these comments? |
@@ -3,5 +3,5 @@ | |||
. InvokeBuildHelperTasks | |||
|
|||
# Build configuration | |||
$IBHConfig.RepositoryTask.Token = Use-VaultSecureString -TargetName 'GitHub Token (claudiospizzi)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is breaking the build script as the module requires do be released before it's actually used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I need to figure out how to back these two out of the pr?
SecurityFever/Functions/Impersonation/Push-ImpersonationContext.ps1
Outdated
Show resolved
Hide resolved
# Used to explicitly set NonInteractive mode | ||
[Parameter(Mandatory = $false)] | ||
[switch] | ||
$NonInteractive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea is good: I will implement this in a dedicated change also including the Use-VaultCredential
which has the same behavior. Also I consider throwing a warning if we were not able to retrieve the credentials in non interactive mode. Should be find for the build system.
@Kagre My bad, didn't submit the review. Should now be visible in this pr. |
Convert the try/catch/finally to use Test-Path instead
on second thought let the error pass through depending on the $ErrorActionPreference if the user chooses to ignore it then they can do so on their end
Take a look, this is a breaking change, but hopefully for the better.
The idea here was to remain in the current on the same working path as long as the new context still had access to it.
Also, if you don't add it in; please at least fix the assumption that when you pop the context back from admin the original user has access to wherever admin wandered off to...