Harden examiner test execution with canonical path validation and strict input handling-Pr 838 base Branch

[Srustisundar]

1. Added strict validation for test names to ensure only simple filenames are accepted.
2. Validated command aguments once and reused the sanitized result to avoid accidental reintroduction of untrusted input.
   3.Resolved executable paths to their canonical (real) filesystem paths before execution.
3. Verified that the canonical path still resides within the allowed examiner directories, preventing symlink-based escapes.
4. Added a defense-in-depth validation before process execution to make intent clear and auditable.

Why this was done
Static analysis flagged the previous implementation due to potential ambiguity around child process execution paths.
Although practical exploitation was already constrained, this hardening removes any remaining doubt by making all trust boundaries explicit also increases the veracode security score

[jorgemoralespou]

hey @Srustisundar All looks fine except for the reformatting of existing code and change in formatting style. But we can fix that in a later PR, since we need to add global formatter

[jorgemoralespou]

@GrahamDumpleton Are you ok with this PR?

[Srustisundar]

Yeah Thats totaly Fine
Thank you so much
For our internal security review, could you please provide a brief justification or risk rationale for this hardening change?
Static analysis previously flagged ambiguity around child process execution paths, and we’d like to document the security reasoning and residual risk.
This will help us complete our audit and compliance review

[GrahamDumpleton]

On first visual pass, I see a few issues with the proposed changes.

First it prohibits a test name including a slash (/) character. There is no need to restrict this and it may break existing workshops which used sub directories under the examiner/tests directory to group related test end points.

```examiner:execute-test
name: my-tests/test-1
```

```examiner:execute-test
name: my-tests/test-2
```

The second is that it should not block test args which are an empty string as that is entirely valid.

```examiner:execute-test
name: my-tests/test-1
args:
- first
- ""
- last
```

Also logging has been removed with something only being logged in some cases when errors occur. Because extra logging cannot be enabled later through config or other means, having general progress logging is important in debugging issues with examiner scripts. The various logging (which was removed), helped delineate in the logs when examiner test scripts were run so is easier to then work out what output to the log from an examiner script related to. So log messages about timeout settings, when test processes were spawned, exit codes etc, should be added back. One cannot just rely on messages only being returned in the json response as that means important information is lost from workshop session log.

[GrahamDumpleton]

One thing to note about directory traversal via .. here, is that the test name is actually appended to /examiner/ path prefix to form the target URL from the front end. Thus, resolving of the ../ is done by browser front end in normal case meaning that trying to use ../ to traverse outside of the directory doesn't do much and would usually result in 404 not found since target URL path would not exist.

This doesn't mean someone couldn't try and target examiner URL directly using curl etc, presuming they also got around auth requirement. So handling of ../ still needs to be blocked in check that target exists under one of the examiner test directories.

Anyway, since various changes were needed from original PR, I am doing it as a separate set of changes which I will commit and push.

[GrahamDumpleton]

Another issue. Although desire that tests can be across sub directories, the URL path handler match string didn't allow that, so need to update that to match whole trailing path of the URL after prefix.

[GrahamDumpleton]

My variant of PR with changes mentioned above, as well as allowing new capability of allowing examiner scripts to be bundled with extension packages.

• Allow examiner scripts in packages and improve checks. #852

[jorgemoralespou]

@Srustisundar Can you validate the above mentioned PR #852 in your static analysis tool, so it complies with your requirements, since we would rather merge the one done by @GrahamDumpleton after a thorough analysis of the code and functionality. We don't want to affect existing users or change expected functionality.