Skip to content

Conversation

@AshokThangavel
Copy link
Contributor

This PR introduces namespace isolation for the exec (and cos) command utility.

Currently, the exec command allows a user to execute arbitrary ObjectScript that can modify the process state. If a user executes a namespace switch (e.g., exec zn "%SYS"), the shell process remains in that new namespace context after the command completes. If that target namespace does not have the %IPM package mapped, the shell immediately crashes when it attempts to render the next prompt or error message because it can no longer find its own configuration classes.

Related Issues

Testing Conducted

  1. Namespace Drift Test: Ran exec zn "%SYS" from the USER namespace.
  • Result: The command executed, and the shell remained in (or returned to) the USER namespace. The prompt rendered correctly.
  1. Error Handling Test: Ran exec set x = 1/0 (divide by zero) to trigger an error.
  • Result: The error was caught, and the namespace was still correctly restored.
  1. Logic Verification: Confirmed that standard exec commands (like exec write 123) still function normally with no overhead.

Copy link
Collaborator

@isc-dchui isc-dchui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor things!

try {
write !
xecute tCommandInfo("parameters","expression")
} catch ex {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just throw the error and let the exception handling of ShellInternal() handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Added the $$$ThrowOnError inside catch block.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simplicity's sake, I don't think we need this extra try/catch block in this method. The error should be propagatable up the call stack, so essentially just

ClassMethod ExecuteCOS(ByRef tCommandInfo)
{
new $namespace
write !
xecute commandInfo("parameters","expression")
}

Also minor nit: remove the t- prefixes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing. I have updated the method to use the minimalist approach as suggested. I removed the try/catch block and the t- prefixes from the variables.


ClassMethod ExecuteCOS(ByRef tCommandInfo)
{
set namespace = $namespace
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a new $namespace line before this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Added the new $namespace and removed the set namespace = $namespace

- #822: The CPF resource processor now supports system expressions and macros in CPF merge files
- #578 Added functionality to record and display IPM history of install, uninstall, load, and update
- #961: Adding creation of a lock file for a module by using the `-create-lockfile` flag on install.
- #996:Ensure COS commands execute in exec under a dedicated, isolated context
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you'll need to resolve conflicts here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved the conflicts

Copy link
Collaborator

@isc-dchui isc-dchui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small test would also be helpful. Shouldn't be too complicated to xecute an erroring expression/code block that switches the namespace and verify the namespace gets reverted back to what it was originally.

try {
write !
xecute tCommandInfo("parameters","expression")
} catch ex {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simplicity's sake, I don't think we need this extra try/catch block in this method. The error should be propagatable up the call stack, so essentially just

ClassMethod ExecuteCOS(ByRef tCommandInfo)
{
new $namespace
write !
xecute commandInfo("parameters","expression")
}

Also minor nit: remove the t- prefixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IPM exec Command Fails in Namespaces Without IPM Mappings After Execution

2 participants