Skip to content

Potential fix for code scanning alert no. 5: Unvalidated dynamic method call#17

Merged
ialejandro merged 4 commits intomainfrom
fix/security-improves
Feb 15, 2026
Merged

Potential fix for code scanning alert no. 5: Unvalidated dynamic method call#17
ialejandro merged 4 commits intomainfrom
fix/security-improves

Conversation

@ialejandro
Copy link
Copy Markdown
Member

Potential fix for https://github.com/devops-ia/self-learning-platform/security/code-scanning/5

In general, to fix unvalidated dynamic method calls, you must ensure that the property name derived from user input is only used to access a vetted set of callable functions. This usually means (a) checking that the name is in a whitelist or Map, (b) confirming the property is an own property (not coming from the prototype chain), and (c) verifying that the retrieved value is a function before calling it. If any check fails, you should handle it gracefully instead of calling the value.

For this specific code, the best minimally invasive fix is to add explicit validation around the dynamic lookups and invocations:

  1. For the exact match on line 26–27:

    • Compute const handler = exercise.terminalCommands[trimmedCommand].
    • Check that Object.prototype.hasOwnProperty.call(exercise.terminalCommands, trimmedCommand) is true, and that typeof handler === "function".
    • Only then call handler(currentCode).
    • Otherwise, fall through to the rest of the logic so that unknown commands are handled by the later built‑in or “command not found” logic.
  2. For the prefix matching loop on lines 31–34:

    • Since Object.entries returns own enumerable properties as [pattern, handler], we already avoid prototype chain issues, but we still must ensure that handler is a function.
    • Add if (typeof handler !== "function") { continue; } before invoking it.
    • Keep the prefix matching logic unchanged apart from this validation.

No new imports are needed; we can use Object.prototype.hasOwnProperty.call directly. All changes are confined to src/lib/terminal/simulator.ts within the executeCommand function.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Iván Alejandro Marugán <hello@ialejandro.rocks>
@ialejandro ialejandro marked this pull request as ready for review February 15, 2026 15:27
: undefined;

if (typeof exactHandler === "function") {
return exactHandler(currentCode);

Check failure

Code scanning / CodeQL

Unvalidated dynamic method call High

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

Copilot Autofix

AI 2 months ago

In general, to fix unvalidated dynamic method calls you must (a) restrict which methods can be called (whitelisting / own-property checks), (b) ensure the resolved property is actually a function before invoking it, and (c) handle any exceptions thrown by those dynamically selected handlers so that they cannot cause an unhandled runtime error or denial of service.

In this code, (a) and (b) are already in place: the exact-match lookup uses hasOwnProperty and a type check, and the prefix loop checks typeof handler === "function". The remaining improvement is to make the dynamic call itself safe by surrounding it with a try/catch so that if a handler throws, the error is translated into a controlled TerminalResponse instead of bubbling up and potentially taking down the request handler. This also helps satisfy CodeQL, which is worried about an exception stemming from a user-controlled dispatch. Concretely, in src/lib/terminal/simulator.ts we should:

  • Wrap exactHandler(currentCode) in a try/catch that returns a friendly error response (for example, using t.terminal.commandError if available, or a generic message) and an appropriate non-zero exit code.
  • Similarly, wrap handler(currentCode) inside the loop in a try/catch with the same behavior.
  • Keep all existing behavior intact when handlers execute successfully.

No new imports are required; we just add the try/catch blocks around the existing calls in executeCommand.


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
@@ -31,7 +31,14 @@
     : undefined;
 
   if (typeof exactHandler === "function") {
-    return exactHandler(currentCode);
+    try {
+      return exactHandler(currentCode);
+    } catch (error) {
+      return {
+        output: `Error: ${t.terminal.commandNotFound}`,
+        exitCode: 1,
+      };
+    }
   }
 
   // Check for command prefix match (e.g., "kubectl logs <pod-name>" matches "kubectl logs")
@@ -40,7 +47,14 @@
       continue;
     }
     if (trimmedCommand.startsWith(pattern) || pattern.startsWith(trimmedCommand)) {
-      return handler(currentCode);
+      try {
+        return handler(currentCode);
+      } catch (error) {
+        return {
+          output: `Error: ${t.terminal.commandNotFound}`,
+          exitCode: 1,
+        };
+      }
     }
   }
 
EOF
@@ -31,7 +31,14 @@
: undefined;

if (typeof exactHandler === "function") {
return exactHandler(currentCode);
try {
return exactHandler(currentCode);
} catch (error) {
return {
output: `Error: ${t.terminal.commandNotFound}`,
exitCode: 1,
};
}
}

// Check for command prefix match (e.g., "kubectl logs <pod-name>" matches "kubectl logs")
@@ -40,7 +47,14 @@
continue;
}
if (trimmedCommand.startsWith(pattern) || pattern.startsWith(trimmedCommand)) {
return handler(currentCode);
try {
return handler(currentCode);
} catch (error) {
return {
output: `Error: ${t.terminal.commandNotFound}`,
exitCode: 1,
};
}
}
}

Copilot is powered by AI and may make mistakes. Always verify output.
ialejandro and others added 3 commits February 15, 2026 16:28
Signed-off-by: Iván Alejandro Marugán <hello@ialejandro.rocks>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Iván Alejandro Marugán <hello@ialejandro.rocks>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Iván Alejandro Marugán <hello@ialejandro.rocks>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@ialejandro ialejandro merged commit 7f271a4 into main Feb 15, 2026
3 of 4 checks passed
@ialejandro ialejandro deleted the fix/security-improves branch February 15, 2026 15:29
github-actions Bot pushed a commit that referenced this pull request Feb 15, 2026
## [1.1.1](v1.1.0...v1.1.1) (2026-02-15)

### Bug Fixes

* Unvalidated dynamic method call ([#17](#17)) ([7f271a4](7f271a4))
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 1.1.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants