From e8b1caaa9d81005d801dd69f0f59ce8d6510dc5c Mon Sep 17 00:00:00 2001 From: Kristina Solovyova Date: Wed, 28 Jan 2026 14:34:39 +0200 Subject: [PATCH] fix: potential command injection, shell injection, path traversal, and secret logging --- .dagger/src/operator_flows/main.py | 8 +- .../dagger_ci_on_merge_queue_plan.yaml | 8 +- .github/workflows/package.yaml | 15 ++- .github/workflows/pr_ai_test.yaml | 100 ------------------ .github/workflows/run_upgrade_test.yaml | 19 ++-- .../weka-operator/resources/weka_runtime.py | 3 + .../wekacontainer/funcs_common_deletion.go | 5 +- .../funcs_resources_allocation.go | 10 +- internal/node_agent/node_agent.go | 6 ++ 9 files changed, 52 insertions(+), 122 deletions(-) delete mode 100644 .github/workflows/pr_ai_test.yaml diff --git a/.dagger/src/operator_flows/main.py b/.dagger/src/operator_flows/main.py index 67e7a54af..178dc65d7 100644 --- a/.dagger/src/operator_flows/main.py +++ b/.dagger/src/operator_flows/main.py @@ -527,14 +527,16 @@ async def copy_k8s_secret_from_one_cluster_to_another( # Create the secret directly using kubectl create secret generic with the data logger.info(f"Creating secret {target_secret_name} in target cluster") - + + # SECURITY NOTE: secret_data is base64-encoded (Kubernetes default) and embedded in heredoc. + # Heredoc prevents shell injection, but avoid logging this command to prevent secret exposure. # Use kubectl to create the secret from the extracted data create_secret_cmd = [ - "sh", "-c", + "sh", "-c", f""" # Delete the secret if it already exists (ignore errors) kubectl --kubeconfig=/.kube/target/config delete secret {target_secret_name} -n {target_namespace} || true - + # Create a temporary YAML file with the secret cat < /tmp/secret.yaml apiVersion: v1 diff --git a/.github/workflows/dagger_ci_on_merge_queue_plan.yaml b/.github/workflows/dagger_ci_on_merge_queue_plan.yaml index 636670ffe..2e2ef0c30 100644 --- a/.github/workflows/dagger_ci_on_merge_queue_plan.yaml +++ b/.github/workflows/dagger_ci_on_merge_queue_plan.yaml @@ -146,7 +146,7 @@ jobs: chmod 600 ${{ github.workspace }}/.kube/config - name: Call Generate Test Artifacts - uses: dagger/dagger-for-github@8.0.0 + uses: dagger/dagger-for-github@v8.2.0 env: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} GH_TOKEN: ${{ steps.app-token.outputs.token }} @@ -171,7 +171,7 @@ jobs: retention-days: 1 - name: Publish Operator - uses: dagger/dagger-for-github@8.0.0 + uses: dagger/dagger-for-github@v8.2.0 env: GH_TOKEN: ${{ steps.app-token.outputs.token }} with: @@ -210,7 +210,7 @@ jobs: find ${{ github.workspace }}/test-artifacts -type f -name "*.sh" -exec ls -la {} \; || echo "No .sh files found" - name: Run Upgrade-extended Test - uses: dagger/dagger-for-github@8.0.0 + uses: dagger/dagger-for-github@v8.2.0 env: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} @@ -295,7 +295,7 @@ jobs: chmod 600 ${{ github.workspace }}/.kube/ocp-cluster-config - name: Run OCP Clients-Only Test - uses: dagger/dagger-for-github@8.0.0 + uses: dagger/dagger-for-github@v8.2.0 env: GH_TOKEN: ${{ steps.app-token.outputs.token }} with: diff --git a/.github/workflows/package.yaml b/.github/workflows/package.yaml index 48b7aae00..5a410ef9e 100644 --- a/.github/workflows/package.yaml +++ b/.github/workflows/package.yaml @@ -99,7 +99,10 @@ jobs: with: version: v3.16.3 - name: Login to Registry - run: helm registry login quay.io/weka.io --username ${{ secrets.QuayUsername }} --password ${{ secrets.QuayPassword }} + env: + QUAY_USERNAME: ${{ secrets.QuayUsername }} + QUAY_PASSWORD: ${{ secrets.QuayPassword }} + run: helm registry login quay.io/weka.io --username "$QUAY_USERNAME" --password "$QUAY_PASSWORD" - name: Configure git run: | git config user.name "$GITHUB_ACTOR" @@ -115,14 +118,18 @@ jobs: - name: Read VERSION from inputs id: version_input if: github.event_name == 'workflow_dispatch' - run: echo "VERSION=${{ github.event.inputs.version-name }}" >> $GITHUB_ENV + env: + VERSION_INPUT: ${{ github.event.inputs.version-name }} + run: echo "VERSION=$VERSION_INPUT" >> $GITHUB_ENV - name: Read VERSION file id: version if: github.event_name != 'workflow_dispatch' run: echo "VERSION=$(cat .VERSION)" >> $GITHUB_ENV - name: build version if: ${{ env.VERSION != '' }} - run: VERSION=${{ env.VERSION }} ./build-release.sh + env: + VERSION: ${{ env.VERSION }} + run: ./build-release.sh - name: Create release if: ${{ env.VERSION != '' && github.event_name != 'workflow_dispatch' }} env: @@ -130,7 +137,7 @@ jobs: run: npm run release - name: Add operator version as artifact if: ${{ env.VERSION != '' }} - run: echo "${{ env.VERSION }}" > operator-version.txt + run: echo "$VERSION" > operator-version.txt - name: Upload artifact if: ${{ env.VERSION != '' }} uses: actions/upload-artifact@v4 diff --git a/.github/workflows/pr_ai_test.yaml b/.github/workflows/pr_ai_test.yaml deleted file mode 100644 index 3cde02d40..000000000 --- a/.github/workflows/pr_ai_test.yaml +++ /dev/null @@ -1,100 +0,0 @@ -name: Run Test with AI hooks (deprecated) -run-name: ${{ github.actor }} is running the AI test - -concurrency: - group: ${{ github.workflow }}-${{ inputs.pr_ids || github.event.pull_request.number }} - cancel-in-progress: false - -permissions: - contents: read - pull-requests: read - actions: read - -on: - # pull_request: - # types: - # - ready_for_review - # branches: - # - main - # workflow_dispatch: - # inputs: - # pr_ids: - # description: 'Space-separated list of PR IDs to process' - # required: true - # type: string - workflow_call: - inputs: - pr_ids: - description: 'Space-separated list of PR IDs to process' - required: true - type: string - outputs: - artifacts_generated: - description: 'Whether artifacts were generated' - value: ${{ jobs.process_prs.outputs.artifacts_generated }} - -jobs: - optimize_ci: - runs-on: ubuntu-latest - outputs: - skip: ${{ steps.check_skip.outputs.skip }} - steps: - - name: Optimize CI - id: check_skip - uses: withgraphite/graphite-ci-action@v0.0.9 - with: - graphite_token: ${{ secrets.GRAPHITE_TOKEN }} - - process_prs: - runs-on: ubuntu-latest - needs: [optimize_ci] - if: ${{ (github.event_name == 'pull_request' && needs.optimize_ci.outputs.skip == 'false') || github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_call' }} - outputs: - artifacts_generated: ${{ steps.check_artifacts.outputs.artifacts_generated }} - steps: - - name: Checkout operator - uses: actions/checkout@v4 - - - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: '3.12' - cache: 'pip' - - name: Install uv - run: pip install uv - - - name: Prepare PR IDs - id: prepare_pr_ids - run: | - if [[ "${{ github.event_name }}" == "pull_request" ]]; then - echo "PR_IDS=${{ github.event.pull_request.number }}" >> $GITHUB_ENV - else - # Use the PR IDs as provided (already space-separated) - echo "PR_IDS=${{ inputs.pr_ids }}" >> $GITHUB_ENV - fi - - - name: Run process_pr_hooks script for all PRs - id: run_process_hooks - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} - GITHUB_PAT_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - # Pass all PR IDs as separate arguments to the script - ./workflows/script_process_pr_hooks.py $PR_IDS - - - name: Check for generated artifacts - id: check_artifacts - run: | - if [ -d "test_artifacts" ] && [ "$(ls -A test_artifacts)" ]; then - echo "artifacts_generated=true" >> $GITHUB_OUTPUT - else - echo "artifacts_generated=false" >> $GITHUB_OUTPUT - fi - - - name: Upload test artifacts - uses: actions/upload-artifact@v4 - with: - name: test-artifacts - path: test_artifacts/ - retention-days: 1 diff --git a/.github/workflows/run_upgrade_test.yaml b/.github/workflows/run_upgrade_test.yaml index e6fc197f5..b1199e40a 100644 --- a/.github/workflows/run_upgrade_test.yaml +++ b/.github/workflows/run_upgrade_test.yaml @@ -146,17 +146,22 @@ jobs: - name: Make binary executable run: chmod +x ./weka-k8s-testing - name: Run upgrade test + env: + INITIAL_VERSION: ${{ github.event.inputs.weka-initial-version || 'quay.io/weka.io/weka-in-container:4.4.5.118-k8s.' }} + NEW_VERSION: ${{ github.event.inputs.weka-new-version || 'quay.io/weka.io/weka-in-container:4.4.10.183' }} + OPERATOR_VERSION_INPUT: ${{ env.OPERATOR_VERSION || github.event.inputs.operator-version }} + DO_CLEANUP: ${{ github.event.inputs.do_cleanup }} run: | cleanup="" - if [[ "${{ github.event.inputs.do_cleanup }}" == "false" ]]; then + if [[ "$DO_CLEANUP" == "false" ]]; then cleanup="--cleanup no-cleanup" fi ./weka-k8s-testing upgrade-extended \ - --initial-version ${{ github.event.inputs.weka-initial-version || 'quay.io/weka.io/weka-in-container:4.4.5.118-k8s.' }} \ - --new-version ${{ github.event.inputs.weka-new-version || 'quay.io/weka.io/weka-in-container:4.4.10.183' }} \ - --operator-version ${{ env.OPERATOR_VERSION || github.event.inputs.operator-version }} \ - --node-selector weka.io/dedicated:upgrade-extended \ - --namespace test-upgrade-extended \ - --cluster-name upgrade-extended \ + --initial-version "$INITIAL_VERSION" \ + --new-version "$NEW_VERSION" \ + --operator-version "$OPERATOR_VERSION_INPUT" \ + --node-selector "weka.io/dedicated:upgrade-extended" \ + --namespace "test-upgrade-extended" \ + --cluster-name "upgrade-extended" \ $cleanup diff --git a/charts/weka-operator/resources/weka_runtime.py b/charts/weka-operator/resources/weka_runtime.py index 19efabceb..54cd743b5 100644 --- a/charts/weka-operator/resources/weka_runtime.py +++ b/charts/weka-operator/resources/weka_runtime.py @@ -2413,6 +2413,9 @@ async def create_container(): {f"--restricted" if MODE == 'client' and "4.2.7.64" not in IMAGE_NAME else ""} \ {f"--failure-domain {failure_domain}" if failure_domain else ""} """) + # join_secret_cmd is a shell command substitution "$(cat /path/to/secret)", not the secret value itself + # The actual secret is read by the shell at runtime and never appears in this logged string + # lgtm[py/clear-text-logging-sensitive-data] logging.info(f"Creating container with command: {command}") stdout, stderr, ec = await run_command(command) if ec != 0: diff --git a/internal/controllers/wekacontainer/funcs_common_deletion.go b/internal/controllers/wekacontainer/funcs_common_deletion.go index 2d6796705..b10c9d75b 100644 --- a/internal/controllers/wekacontainer/funcs_common_deletion.go +++ b/internal/controllers/wekacontainer/funcs_common_deletion.go @@ -3,6 +3,7 @@ package wekacontainer import ( "context" + "encoding/base64" "encoding/json" "fmt" "path" @@ -232,7 +233,9 @@ func (r *containerReconcilerLoop) sendStopInstructionsViaAgent(ctx context.Conte instructionsBasePath := path.Join(resources.GetPodShutdownInstructionPathOnAgent(nodeInfo.BootID, pod)) instructionsPath := path.Join(instructionsBasePath, "shutdown_instructions.json") - _, _, err = executor.ExecNamed(ctx, "StopInstructionsViaAgent", []string{"bash", "-ce", fmt.Sprintf("mkdir -p '%s' && echo '%s' > '%s'", instructionsBasePath, instructionsJson, instructionsPath)}) + // Use base64 encoding to safely pass JSON through shell + instructionsB64 := base64.StdEncoding.EncodeToString(instructionsJson) + _, _, err = executor.ExecNamed(ctx, "StopInstructionsViaAgent", []string{"bash", "-ce", fmt.Sprintf("mkdir -p '%s' && echo '%s' | base64 -d > '%s'", instructionsBasePath, instructionsB64, instructionsPath)}) if err != nil { logger.Error(err, "Error writing stop instructions via node-agent") return err diff --git a/internal/controllers/wekacontainer/funcs_resources_allocation.go b/internal/controllers/wekacontainer/funcs_resources_allocation.go index 9ffc5282e..f31fd8ebc 100644 --- a/internal/controllers/wekacontainer/funcs_resources_allocation.go +++ b/internal/controllers/wekacontainer/funcs_resources_allocation.go @@ -4,6 +4,7 @@ package wekacontainer import ( "context" + "encoding/base64" "encoding/json" "fmt" "go/types" @@ -174,14 +175,17 @@ func (r *containerReconcilerLoop) WriteResources(ctx context.Context) error { return err } - // Use base64 encoding for safe file writing - prevents bash injection issues + // Use base64 encoding to safely pass JSON through shell + // JSON can contain single quotes in string values, + // which would break echo 'JSON' shell command and allow potential code injection resourcesStr := string(resourcesJson) logger.Info("writing resources", "json", resourcesStr) + resourcesB64 := base64.StdEncoding.EncodeToString(resourcesJson) stdout, stderr, err := executor.ExecNamed(ctx, "WriteResources", []string{"bash", "-ce", fmt.Sprintf(` mkdir -p /opt/weka/k8s-runtime/tmp -echo '%s' > /opt/weka/k8s-runtime/tmp/resources.json +echo '%s' | base64 -d > /opt/weka/k8s-runtime/tmp/resources.json mv /opt/weka/k8s-runtime/tmp/resources.json /opt/weka/k8s-runtime/resources.json -`, resourcesStr)}) +`, resourcesB64)}) if err != nil { logger.Error(err, "Error writing resources", "stderr", stderr.String(), "stdout", stdout.String()) return err diff --git a/internal/node_agent/node_agent.go b/internal/node_agent/node_agent.go index 268b38c68..71f28244e 100644 --- a/internal/node_agent/node_agent.go +++ b/internal/node_agent/node_agent.go @@ -1119,6 +1119,12 @@ func (a *NodeAgent) getActiveMounts(w http.ResponseWriter, r *http.Request) { } if shouldCheckContainerSpecific { + // Validate containerName to prevent path traversal attacks + // Container names should only contain alphanumeric characters, hyphens, and underscores + if strings.Contains(containerName, "..") || strings.Contains(containerName, "/") || strings.Contains(containerName, "\\") { + http.Error(w, "invalid container name", http.StatusBadRequest) + return + } filePath = fmt.Sprintf("/proc/wekafs/%s/interface", containerName) } else { // Feature flag not enabled, use default path