Strengthen security posture and code quality standards - #126
Open
sgurunat wants to merge 1 commit into
Open
Conversation
- Use AST-based arithmetic evaluation in MCP example servers - Validate URL schemes explicitly before remote downloads - Align all shell scripts with ShellCheck recommendations - Add SPDX and copyright headers where missing - Source deployment placeholder values from the environment - Use curl --noproxy for in-cluster Kubernetes API calls
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Repo-wide hardening and code-quality pass across shell scripts and Python examples, plus a fix for in-cluster Kubernetes API calls on proxied hosts. No functional or API changes to deployment behavior.
What changed
Python examples (MCP servers)
Arithmetic in the calculate tool of all five example servers (airline, banking, retail, stocks, triage) now goes through a shared AST-based evaluator that accepts only numeric literals and + - * /.
Remote dataset/HTTP fetches validate the URL scheme before the request and apply an explicit timeout.
Shell scripts (66 files)
All scripts now align with ShellCheck recommendations: quoted expansions, mapfile in place of unquoted command substitution into arrays, printf '%b' for escape sequences, direct exit-status tests instead of $?, and cd … || exit.
Sourced library fragments under core/lib/** declare # shellcheck shell=bash and document the shared-globals contract with lib/system/config-vars.sh.
SPDX and copyright headers added where missing — now present in all 66 scripts.
Configuration handling
Placeholder values in example and deploy scripts are sourced from the environment with the same defaults as before, so no credentials are carried in tracked files.
third_party/Dell/**/iac/iso/custom-iso.sh takes its password hash exclusively from --password-hash, which the script already documented as required.
Dropped two unused connection-string variables from core/scripts/generate-vault-secrets.sh.
Proxy-aware Kubernetes API access
core/scripts/keycloak-realmcreation.sh passes curl --noproxy "$KUBERNETES_SERVICE_HOST" on its three API-server calls. On hosts with http_proxy/https_proxy set, older curl builds do not match CIDR entries in no_proxy (e.g. 10.233.0.0/16), so in-cluster requests were routed to the external proxy. --noproxy takes an exact host and works across curl versions; it is a no-op when no proxy is configured.
New files, and why
sample_solutions/AgenticCodeExecution/examples/safe_math.py - Single shared arithmetic evaluator for all five MCP servers, rather than duplicating the same logic five times. Placed alongside the existing error_hints.py shared module and picked up by the images automatically via the Dockerfile's COPY examples /app/examples.
.shellcheckrc -- Sets external-sources=true / source-path=SCRIPTDIR so ShellCheck resolves source ./lib/… from the repo root the way the scripts do at runtime. This eliminates unresolved-source reports at their root cause and lets genuine issues in sourced files surface, instead of blanket-suppressing them.
Behavior notes
core/scripts/generate-token.sh requires KEYCLOAK_PASSWORD to be set and exits with a clear message otherwise; the previous placeholder default could not authenticate against a live Keycloak. docs/getting-started-deploy-MCP.md is updated to match.
Fixed alongside the cleanup: retry paths in cluster-update.sh and user-menu.sh now forward "$@" instead of dropping arguments; compute_reserved_cpus.sh no longer leaves IFS globally modified; load_configuration in vllm-model-runner.sh reports jq parse failures instead of continuing with an empty model list.
Validation
Bandit (-r . -ll -iii) and ShellCheck (-f gcc, all 66 scripts): clean.
ClamAV: clean, unchanged.
bash -n passes on every modified script; all touched Python compiles.
Output equivalence verified by execution for the arithmetic evaluator, compute_reserved_cpus.sh (n = 1, 3, 4, 8, 16), execute_and_check, the vllm-model-runner.sh sudo invocation, and custom-iso.sh argument validation.
End-to-end inference verified on a deployed cluster.