Skip to content

Potential fix for code scanning alert no. 6: Unvalidated dynamic method call #21

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.

Sign up for GitHub

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

Merged
ialejandro merged 2 commits into from
Feb 15, 2026
Merged

Potential fix for code scanning alert no. 6: Unvalidated dynamic method call #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b5f7ae
fix: Unvalidated dynamic method call
ialejandro Feb 15, 2026
0935bfb
ci: Remove permissions from lint-test workflow (#22)
ialejandro Feb 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/lint-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ on:
pull_request:
branches: [main]

permissions:
contents: read

jobs:
lint:
name: Lint (Node ${{ matrix.node-version }})
Expand Down
26 changes: 17 additions & 9 deletions src/lib/terminal/simulator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@

const trimmedCommand = command.trim();

// Selected handler to execute after validation
let handlerToExecute: ((code: string) => TerminalResponse) | undefined;

// Check for exact match first (ensure own property and handler is a function)
const exactHandler = Object.prototype.hasOwnProperty.call(
exercise.terminalCommands,
Expand All @@ -31,17 +34,22 @@
: undefined;

if (typeof exactHandler === "function") {
return exactHandler(currentCode);
handlerToExecute = exactHandler;
} else {
// Check for command prefix match (e.g., "kubectl logs <pod-name>" matches "kubectl logs")
for (const [pattern, handler] of Object.entries(exercise.terminalCommands)) {
if (typeof handler !== "function") {
continue;
}
if (trimmedCommand.startsWith(pattern) || pattern.startsWith(trimmedCommand)) {
handlerToExecute = handler;
break;
}
}
}

// Check for command prefix match (e.g., "kubectl logs <pod-name>" matches "kubectl logs")
for (const [pattern, handler] of Object.entries(exercise.terminalCommands)) {
if (typeof handler !== "function") {
continue;
}
if (trimmedCommand.startsWith(pattern) || pattern.startsWith(trimmedCommand)) {
return handler(currentCode);
}
if (handlerToExecute) {
return handlerToExecute(currentCode);

Check failure

Code scanning / CodeQL

Unvalidated dynamic method call High

Invocation of method with
user-controlled
Loading
name may dispatch to unexpected target and cause an exception.

Copilot Autofix

AI 5 days ago

In general, to fix this kind of issue you must ensure that user-controlled strings cannot arbitrarily select or invoke unexpected methods. This is typically done by (a) restricting dispatch to a known whitelist of commands, (b) verifying that the selected value is an own property and a function, and (c) avoiding overly permissive matching logic that might invoke commands on partial matches in surprising ways. It’s also good practice to handle exceptions from invoked handlers so one bad handler or input cannot take down the whole endpoint.

For this specific code, the essential parts are already in place: an own-property check and a typeof === "function" check. The main remaining concerns are: (1) the looser prefix-matching path, and (2) the lack of a safety net around the handler call. To keep current behavior while improving safety, we can:

  1. Keep the exact-match logic as is (it already validates own property and function type).
  2. Keep the prefix-matching logic but explicitly ensure we’re only iterating over own properties (using Object.entries is fine) and already checking typeof handler === "function"; that’s good.
  3. Add a defensive guard before invocation to verify typeof handlerToExecute === "function" right at the call site; this guards against future changes where handlerToExecute might accidentally be set to a non-function.
  4. Wrap the invocation in a try/catch block so unexpected exceptions from handler code cannot crash the API; instead, return a controlled error message and non-zero exit code. This directly mitigates the DoS angle CodeQL is concerned about, without changing which commands are permitted.

All necessary changes are in src/lib/terminal/simulator.ts, around the final check and invocation of handlerToExecute. No import changes or new helper functions are required.

Suggested changeset 1
src/lib/terminal/simulator.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/lib/terminal/simulator.ts b/src/lib/terminal/simulator.ts
--- a/src/lib/terminal/simulator.ts
+++ b/src/lib/terminal/simulator.ts
@@ -48,8 +48,15 @@
     }
   }
 
-  if (handlerToExecute) {
-    return handlerToExecute(currentCode);
+  if (typeof handlerToExecute === "function") {
+    try {
+      return handlerToExecute(currentCode);
+    } catch (error) {
+      return {
+        output: `Error: command execution failed`,
+        exitCode: 1,
+      };
+    }
   }
 
   // Built-in commands
EOF
@@ -48,8 +48,15 @@
}
}

if (handlerToExecute) {
return handlerToExecute(currentCode);
if (typeof handlerToExecute === "function") {
try {
return handlerToExecute(currentCode);
} catch (error) {
return {
output: `Error: command execution failed`,
exitCode: 1,
};
}
}

// Built-in commands
Copilot is powered by AI and may make mistakes. Always verify output.
}

// Built-in commands
Expand Down
Loading