Address review: SP-mediated uploads, login_required, platform binary, code smell fix

Author: pulk17

install/ci-vm/ci-linux/ci/runCI

@@ -139,10 +139,11 @@ if [ -e "${dstDir}/ccextractor" ]; then
 #!/bin/bash
 COMBINED_LOG="/tmp/combined_stdout.log"
 REAL_BINARY="PLACEHOLDER_BINARY"
-EXIT_CODE_FILE="/tmp/.wrapper_exit_code"
+EXIT_CODE_FILE=$(mktemp)
 echo "=== TEST INVOCATION: $@ ===" >> "$COMBINED_LOG"
 { "$REAL_BINARY" "$@" 2>&1; echo $? > "$EXIT_CODE_FILE"; } | tee -a "$COMBINED_LOG"
 exit_code=$(cat "$EXIT_CODE_FILE")
+rm -f "$EXIT_CODE_FILE"
 echo "=== EXIT CODE: ${exit_code} ===" >> "$COMBINED_LOG"
 echo "" >> "$COMBINED_LOG"
 exit $exit_code
@@ -153,37 +154,33 @@ WRAPPER_EOF
         executeCommand cd ${suiteDstDir}
         executeCommand ${tester} --debug --entries "${testFile}" --executable "${wrapper_path}" --tempfolder "${tempFolder}" --timeout 600 --reportfolder "${reportFolder}" --resultfolder "${resultFolder}" --samplefolder "${sampleFolder}" --method Server --url "${reportURL}"
 
-        # Upload AI artifacts to GCS
-        gcs_bucket=$(curl -s "http://metadata/computeMetadata/v1/instance/attributes/bucket" -H "Metadata-Flavor: Google")
-        test_id=$(curl -s "http://metadata/computeMetadata/v1/instance/attributes/testID" -H "Metadata-Flavor: Google")
-        token=$(curl -s "http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/token" -H "Metadata-Flavor: Google" | python3 -c "import sys,json; print(json.load(sys.stdin)['access_token'])")
-
+        # Upload artifacts through the Sample Platform server
         upload_artifact() {
             local file_path="$1"
-            local dest_path="$2"
+            local artifact_name="$2"
             if [ -f "$file_path" ]; then
                 local http_code
-                http_code=$(curl -s -X POST --data-binary @"$file_path" \
-                    -H "Authorization: Bearer $token" \
-                    -H "Content-Type: application/octet-stream" \
-                    -w "%{http_code}" \
-                    -o /dev/null \
-                    "https://storage.googleapis.com/upload/storage/v1/b/${gcs_bucket}/o?uploadType=media&name=${dest_path}")
-                if [ -z "$http_code" ] || [ "$http_code" -ne 200 ]; then
-                    echo "GCS upload failed for ${dest_path}: HTTP ${http_code:-no_response}" >> "${logFile}"
+                http_code=$(curl -s -A "${userAgent}" \
+                    --form "type=artifact" \
+                    --form "name=${artifact_name}" \
+                    --form "file=@${file_path}" \
+                    -w "%{http_code}" -o /dev/null \
+                    "${reportURL}" 2>/dev/null)
+                if [ -z "$http_code" ] || [ "$http_code" -lt 200 ] || [ "$http_code" -ge 300 ]; then
+                    echo "Artifact upload failed for ${artifact_name}: HTTP ${http_code:-no_response}" >> "${logFile}"
                 fi
             fi
         }
 
-        upload_artifact "$ccextractor_path" "test_artifacts/${test_id}/ccextractor"
+        upload_artifact "$ccextractor_path" "ccextractor"
 
         # Upload combined stdout log
-        upload_artifact "${combined_stdout}" "test_artifacts/${test_id}/combined_stdout.log"
+        upload_artifact "${combined_stdout}" "combined_stdout.log"
 
         # Upload coredumps if any
         for core_file in /tmp/coredumps/core.*; do
             if [ -f "$core_file" ]; then
-                upload_artifact "$core_file" "test_artifacts/${test_id}/coredump"
+                upload_artifact "$core_file" "coredump"
                 break
             fi
         done

mod_ci/controllers.py

@@ -1194,9 +1194,7 @@ def create_instance(compute, project, zone, test, reportURL) -> Dict:
             startup_script = f.read()
         metadata_items = [
             {'key': 'startup-script', 'value': startup_script},
-            {'key': 'reportURL', 'value': reportURL},
-            {'key': 'bucket', 'value': config.get('GCS_BUCKET_NAME', '')},
-            {'key': 'testID', 'value': str(test.id)}
+            {'key': 'reportURL', 'value': reportURL}
         ]
     elif test.platform == TestPlatform.windows:
         image_response = compute.images().getFromFamily(project=config.get('WINDOWS_INSTANCE_PROJECT_NAME', ''),
@@ -1217,9 +1215,7 @@ def create_instance(compute, project, zone, test, reportURL) -> Dict:
             {'key': 'windows-startup-script-ps1', 'value': startup_script},
             {'key': 'service_account', 'value': service_account},
             {'key': 'rclone_conf', 'value': rclone_conf},
-            {'key': 'reportURL', 'value': reportURL},
-            {'key': 'bucket', 'value': config.get('GCS_BUCKET_NAME', '')},
-            {'key': 'testID', 'value': str(test.id)}
+            {'key': 'reportURL', 'value': reportURL}
         ]
     source_disk_image = image_response['selfLink']
 
@@ -2346,6 +2342,11 @@ def progress_reporter(test_id, token):
                 if not upload_type_request(log, test_id, repo_folder, test, request):
                     return "EMPTY"
 
+            elif request.form['type'] == 'artifact':
+                log.info(f'[PROGRESS_REPORTER][Test: {test_id}] Artifact upload')
+                if not artifact_upload_request(log, test_id, request):
+                    return "EMPTY"
+
             elif request.form['type'] == 'finish':
                 log.info(f'[PROGRESS_REPORTER][Test: {test_id}] Test finished')
                 finish_type_request(log, test_id, test, request)
@@ -2695,6 +2696,45 @@ def upload_type_request(log, test_id, repo_folder, test, request) -> bool:
     return False
 
 
+# Allowed artifact names that the VM can upload
+ALLOWED_ARTIFACT_NAMES = {'ccextractor', 'ccextractor.exe', 'combined_stdout.log', 'coredump'}
+
+
+def artifact_upload_request(log, test_id, request) -> bool:
+    """
+    Handle artifact upload from the CI VM.
+
+    Validates the artifact name against an allow-list, then uploads
+    the file to GCS under test_artifacts/{test_id}/{name}.
+
+    :param log: logger
+    :type log: Logger
+    :param test_id: The id of the test to update.
+    :type test_id: int
+    :param request: Request parameters
+    :type request: Request
+    :return: True if upload succeeded, False otherwise.
+    :rtype: bool
+    """
+    from run import storage_client_bucket
+
+    artifact_name = request.form.get('name', '')
+    if artifact_name not in ALLOWED_ARTIFACT_NAMES:
+        log.warning(f"[Test: {test_id}] Rejected artifact upload with disallowed name: {artifact_name}")
+        return False
+
+    if 'file' not in request.files:
+        log.warning(f"[Test: {test_id}] Artifact upload missing file")
+        return False
+
+    uploaded_file = request.files['file']
+    blob_path = f'test_artifacts/{test_id}/{artifact_name}'
+    blob = storage_client_bucket.blob(blob_path)
+    blob.upload_from_file(uploaded_file.stream)
+    log.info(f"[Test: {test_id}] Artifact '{artifact_name}' uploaded to {blob_path}")
+    return True
+
+
 def finish_type_request(log, test_id, test, request):
     """
     Handle finish request type for progress reporter.