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.
This commit is contained in:
benjamin.baker
2026-09-04 16:19:28 -04:00
parent b973561cb7
commit d3f69bbb71
3 changed files with 94 additions and 14 deletions

View File

@@ -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)

View File

@@ -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 = ''

View File

@@ -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']