From d3f69bbb71600fc75c9f821444b72e875a49daf6 Mon Sep 17 00:00:00 2001
From: "benjamin.baker" <benjamin.baker@geaerospace.com>
Date: Fri, 4 Sep 2026 16:19:28 -0400
Subject: [PATCH] Trust the binary PE version over the typed agent version

Uploading an agent build whose product version carried semver build metadata
(1.5.0+abcdef) was rejected, because the typed version string was validated as
a dotted-numeric value before the file was ever read. That string is only a
hint from the operator, and the build metadata it carries is not a legal
directory name for the version store.

The authority is the dotted-numeric FILE version stamped in the binary PE
resource, so validation now happens after the upload is parsed: when the typed
value is missing or not dotted-numeric, fall back to the detected version, and
reject only when neither yields a usable one.
---
 plugins/endpoints/api/agentupload.py          | 21 ++++--
 .../frontend/views/AgentKeySettings.vue       | 22 ++++---
 .../test_endpoints_agentupload.py             | 65 +++++++++++++++++++
 3 files changed, 94 insertions(+), 14 deletions(-)
 create mode 100644 tests/test_plugins/test_endpoints_agentupload.py

diff --git a/plugins/endpoints/api/agentupload.py b/plugins/endpoints/api/agentupload.py
index ae463ab5..bc1a9fce 100644
--- a/plugins/endpoints/api/agentupload.py
+++ b/plugins/endpoints/api/agentupload.py
@@ -69,11 +69,6 @@ def upload_agent_binary(binkey):
             ErrorCodes.VALIDATION_ERROR,
             'binkey must be one of: agent, tray',
             http_code=400)
-    if not agentstore.is_valid_version(version):
-        return error_response(
-            ErrorCodes.VALIDATION_ERROR,
-            'version must be a dotted-numeric string such as 1.6.3',
-            http_code=400)
 
     upload = request.files.get('file')
     if upload is None or not upload.filename:
@@ -94,6 +89,22 @@ def upload_agent_binary(binkey):
             'uploaded file is not a Windows executable',
             http_code=400)
 
+    # Gate the version at the PE, not at the raw input. Product versions carry
+    # semver build metadata (1.5.0+abcdef...) that is not a legal directory
+    # name, so the operator's typed string is only a hint. The dotted-numeric
+    # FILE version stamped in the binary's PE resource is the authority; fall
+    # back to it whenever the typed value is missing or not dotted-numeric, and
+    # only reject when neither yields a usable version.
+    if not agentstore.is_valid_version(version):
+        detected = agentstore.read_pe_file_version(data)
+        if agentstore.is_valid_version(detected):
+            version = detected
+        else:
+            return error_response(
+                ErrorCodes.VALIDATION_ERROR,
+                'version must be a dotted-numeric string such as 1.6.3',
+                http_code=400)
+
     result = agentstore.store_version(binkey, version, data,
                                       make_current=makecurrent)
 
diff --git a/plugins/endpoints/frontend/views/AgentKeySettings.vue b/plugins/endpoints/frontend/views/AgentKeySettings.vue
index a3b659f3..f0d78107 100644
--- a/plugins/endpoints/frontend/views/AgentKeySettings.vue
+++ b/plugins/endpoints/frontend/views/AgentKeySettings.vue
@@ -355,7 +355,7 @@
             <button
               class="btn btn-secondary"
               type="button"
-              :disabled="binary.busy || !binary.version || !binary.file"
+              :disabled="binary.busy || binary.detecting || !binary.file"
               @click="uploadBinary(binary.key)"
             >
               {{ binary.busy ? 'Uploading...' : 'Upload current release' }}
@@ -370,8 +370,9 @@
             Upload legacy build for rollback
           </label>
           <p class="field-warning">
-            WARNING: confirm agent version before uploading from the EXE
-            properties and enter the version number.
+            The version is read from the EXE itself. Leave this blank to use the
+            binary's own file version, or enter a dotted-numeric override only
+            when the build carries no readable version.
           </p>
           <div class="binary-upload">
             <input
@@ -389,7 +390,7 @@
             <button
               class="btn btn-secondary"
               type="button"
-              :disabled="binary.busy || !binary.legacyversion || !binary.legacyfile"
+              :disabled="binary.busy || !binary.legacyfile"
               @click="uploadLegacy(binary.key)"
             >
               {{ binary.busy ? 'Uploading...' : 'Upload legacy build' }}
@@ -756,7 +757,8 @@ async function detectVersion(binkey, file) {
   const form = new FormData()
   form.append('file', file)
   const response = await api.post(
-    `/endpoints/siteagent/binaries/${binkey}/detect`, form)
+    `/endpoints/siteagent/binaries/${binkey}/detect`, form,
+    { headers: { 'Content-Type': 'multipart/form-data' } })
   return response.data.data?.version || ''
 }
 
@@ -789,7 +791,7 @@ function onLegacyFileChange(binkey, event) {
 
 async function uploadBinary(binkey) {
   const binary = binaryFor(binkey)
-  if (!binary || !binary.file || !binary.version) return
+  if (!binary || !binary.file) return
   binary.busy = true
   binary.message = ''
   binary.error = ''
@@ -798,7 +800,8 @@ async function uploadBinary(binkey) {
     form.append('version', binary.version)
     form.append('file', binary.file)
     const response = await api.post(
-      `/endpoints/siteagent/binaries/${binkey}`, form)
+      `/endpoints/siteagent/binaries/${binkey}`, form,
+      { headers: { 'Content-Type': 'multipart/form-data' } })
     const data = response.data.data || {}
     applyBinaryStatus(binkey, data.current, data.versions)
     binary.version = ''
@@ -817,7 +820,7 @@ async function uploadBinary(binkey) {
 
 async function uploadLegacy(binkey) {
   const binary = binaryFor(binkey)
-  if (!binary || !binary.legacyfile || !binary.legacyversion) return
+  if (!binary || !binary.legacyfile) return
   binary.busy = true
   binary.message = ''
   binary.error = ''
@@ -827,7 +830,8 @@ async function uploadLegacy(binkey) {
     form.append('file', binary.legacyfile)
     form.append('makecurrent', 'false')
     const response = await api.post(
-      `/endpoints/siteagent/binaries/${binkey}`, form)
+      `/endpoints/siteagent/binaries/${binkey}`, form,
+      { headers: { 'Content-Type': 'multipart/form-data' } })
     const data = response.data.data || {}
     applyBinaryStatus(binkey, data.current, data.versions)
     binary.legacyversion = ''
diff --git a/tests/test_plugins/test_endpoints_agentupload.py b/tests/test_plugins/test_endpoints_agentupload.py
new file mode 100644
index 00000000..63b51b82
--- /dev/null
+++ b/tests/test_plugins/test_endpoints_agentupload.py
@@ -0,0 +1,65 @@
+"""Agent binary upload version gating: PE-fallback and precedence.
+
+The upload route no longer rejects the version at raw input. A typed value is
+only a hint; the dotted-numeric FILE version in the binary's PE resource is the
+authority. The route falls back to it when the typed value is missing or not
+dotted-numeric, and only refuses when neither yields a usable version.
+
+Building a real PE with a RT_VERSION resource inline is impractical (see the
+agentstore unit tests), so these API tests monkeypatch read_pe_file_version to
+stand in for the parser. PE-magic bytes keep looks_like_pe happy so the request
+reaches the version gate.
+"""
+
+import io
+
+from plugins.endpoints.api import agentstore
+
+UPLOAD_URL = '/api/endpoints/siteagent/binaries/{}'
+
+_PE_BYTES = b'MZ' + b'\x00' * 128
+
+
+def _upload(client, headers, monkeypatch, detected, typed=None):
+    monkeypatch.setattr(agentstore, 'read_pe_file_version',
+                        lambda data: detected)
+    form = {'file': (io.BytesIO(_PE_BYTES), 'shopdbagent.exe')}
+    if typed is not None:
+        form['version'] = typed
+    return client.post(
+        UPLOAD_URL.format('agent'),
+        data=form,
+        content_type='multipart/form-data',
+        headers=headers)
+
+
+def test_upload_falls_back_to_pe_version_when_typed_is_missing(
+        client, db, app, auth_headers, monkeypatch):
+    response = _upload(client, auth_headers, monkeypatch, '1.5.0.0')
+    assert response.status_code == 200, response.get_json()
+    payload = response.get_json()['data']
+    assert payload['current'] == '1.5.0.0'
+    assert any(v['version'] == '1.5.0.0' for v in payload['versions'])
+
+
+def test_upload_falls_back_when_typed_is_not_dotted_numeric(
+        client, db, app, auth_headers, monkeypatch):
+    response = _upload(client, auth_headers, monkeypatch, '1.5.0.0',
+                       typed='1.5.0+1a1bf0640f956634dada1b34840')
+    assert response.status_code == 200, response.get_json()
+    assert response.get_json()['data']['current'] == '1.5.0.0'
+
+
+def test_upload_prefers_valid_typed_version_over_pe(
+        client, db, app, auth_headers, monkeypatch):
+    response = _upload(client, auth_headers, monkeypatch, '9.9.9.9',
+                       typed='1.6.3')
+    assert response.status_code == 200, response.get_json()
+    assert response.get_json()['data']['current'] == '1.6.3'
+
+
+def test_upload_rejects_when_neither_typed_nor_pe_is_usable(
+        client, db, app, auth_headers, monkeypatch):
+    response = _upload(client, auth_headers, monkeypatch, None)
+    assert response.status_code == 400
+    assert 'dotted-numeric' in response.get_json()['data']['error']['message']