Hard delete applications, nest installer packages, detect PE versions

Soft delete left orphaned executables on disk and let a deleted name collide
with a fresh import, so delete is now a hard delete that removes the
associated files. The destructive paths (delete, revert-to-record) require a
typed OK because neither can be undone.

Installers keep their filename at import and nest per application/version,
which stops repeat imports doubling up. Version now comes from the same PE
detection the endpoints agent uses; when no version can be read the caller is
asked for one rather than silently storing a blank.

Applications also surface Version and a monitorable toggle, with launch path
and process name required only while monitorable is ticked. Fleet shows the
PC alias, matching PC Details.
This commit is contained in:
benjamin.baker
2026-09-07 15:04:21 -04:00
parent 79545db96e
commit 0134eaa219
17 changed files with 1600 additions and 148 deletions

View File

@@ -762,8 +762,16 @@ export const applicationsApi = {
update(id, data) {
return api.put(`/applications/${id}`, data)
},
delete(id) {
return api.delete(`/applications/${id}`)
// Hard delete. The backend requires the operator's typed confirmation in the
// body, so the caller must pass what the user actually typed - this is not
// defaulted here, or the guard would be meaningless.
delete(id, confirm) {
return api.delete(`/applications/${id}`, { data: { confirm } })
},
// Strips policies, versions and uploaded executables but keeps the catalogue
// row and its image. Same typed-confirmation contract as delete.
revertToRecord(id, confirm) {
return api.post(`/applications/${id}/revert`, { confirm })
},
// multipart image upload; backend sets image to the served URL
uploadImage(id, file) {

View File

@@ -0,0 +1,86 @@
"""Purge soft-deleted applications that still reserve their names.
Delete used to set isactive=False and leave the row in place. The uniqueness
check on create looks at every row regardless of isactive, so each of those
rows kept its name reserved permanently: creating an application with the same
name failed with "already exists" against a row no screen would display, and
nothing in the UI could clear it.
Delete is a hard delete now, but that only fixes rows deleted from here on.
This clears the ones already stranded, in every environment rather than just
the one where the bug was noticed.
Dependent rows are cleared the same way the API does it, and for the same
reasons: appversions and computerinstalledapps are NOT NULL and meaningless
without their application, while knowledgebase.appid is nullable and the
article is somebody's writing, so it is unlinked rather than destroyed.
Uploaded installers belonging to these rows are deliberately NOT touched. A
migration that deletes files cannot be rolled back and cannot be dry-run, and a
stray file is harmless next to that risk. Deletes made through the API do clean
them up.
Revision ID: 7d45_purge_inactive_apps
Revises: 7d44_app_monitorable
"""
from alembic import op
import sqlalchemy as sa
revision = '7d45_purge_inactive_apps'
down_revision = '7d44_app_monitorable'
branch_labels = None
depends_on = None
def _table_exists(bind, name):
"""True when a table is present.
The plugin tables are only there when the plugin is installed, so each is
checked before being touched.
"""
return sa.inspect(bind).has_table(name)
def upgrade():
bind = op.get_bind()
if not _table_exists(bind, 'applications'):
return
doomed = [row[0] for row in bind.execute(sa.text(
'SELECT appid FROM applications WHERE isactive = false'
))]
if not doomed:
return
for appid in doomed:
params = {'appid': appid}
if _table_exists(bind, 'knowledgebase'):
bind.execute(sa.text(
'UPDATE knowledgebase SET appid = NULL WHERE appid = :appid'
), params)
if _table_exists(bind, 'computerinstalledapps'):
bind.execute(sa.text(
'DELETE FROM computerinstalledapps WHERE appid = :appid'
), params)
if _table_exists(bind, 'appversions'):
bind.execute(sa.text(
'DELETE FROM appversions WHERE appid = :appid'
), params)
bind.execute(sa.text(
'DELETE FROM applications WHERE appid = :appid'
), params)
def downgrade():
# The rows are gone and their contents were not captured, so there is
# nothing to restore. Failing loudly is better than pretending otherwise.
raise NotImplementedError(
'Purged applications cannot be restored; recover from a backup.')

View File

@@ -0,0 +1,136 @@
"""Move uploaded installers into per-application directories.
Installers used to be stored flat as "application-<id>.<ext>", which threw away
the name the vendor shipped. That is what made a re-import look like new
software: the operator had no way to see that the file already in the catalogue
was the same one they were about to upload again, so duplicate applications and
versions accumulated. Installers now keep their filename, namespaced by
application id so a generic "setup.exe" from two vendors cannot collide.
That only helps files uploaded from here on. This moves the ones already stored,
so existing download links keep working instead of 404ing against a path the
route no longer serves.
The original filename cannot be recovered - the old scheme discarded it at
upload - so these land as "installer.<ext>". That is no worse than what they
were called before, and re-uploading the installer restores the real name.
Files are MOVED, not copied or deleted: a move is reversible, which is what
lets downgrade() put them back. This is deliberately unlike 7d45, which refused
to touch files because it could only have deleted them.
The database is the part that must not be left inconsistent, so installpath is
rewritten for every affected row whether or not its file was found on disk. A
row pointing at a missing file is a broken download; a row pointing at the old
scheme is a broken download that also cannot be repaired by re-uploading.
Revision ID: 7d46_nest_app_packages
Revises: 7d45_purge_inactive_apps
"""
import os
import re
from alembic import op
import sqlalchemy as sa
revision = '7d46_nest_app_packages'
down_revision = '7d45_purge_inactive_apps'
branch_labels = None
depends_on = None
OLD_PREFIX = '/api/applications/package/'
# application-<id>.<ext>, the only shape the old uploader produced.
OLD_NAME = re.compile(r'^application-(\d+)\.([A-Za-z0-9]+)$')
def _package_dir():
"""The instance-relative directory installers are stored in.
Resolved the same way the API resolves it. If the app context is not
available the migration still rewrites the database and simply leaves the
files alone, rather than failing the whole upgrade over a path lookup.
"""
try:
from flask import current_app
return os.path.join(current_app.instance_path, 'applicationpackages')
except Exception:
return None
def upgrade():
bind = op.get_bind()
rows = bind.execute(sa.text(
"SELECT appid, installpath FROM applications "
"WHERE installpath LIKE :prefix"
), {'prefix': OLD_PREFIX + '%'}).fetchall()
directory = _package_dir()
for appid, installpath in rows:
tail = installpath[len(OLD_PREFIX):]
match = OLD_NAME.match(tail)
if not match:
# Already nested (contains a slash) or otherwise not the old shape.
# Leaving it alone is correct: it is already being served.
continue
ext = match.group(2).lower()
stored = f'installer.{ext}'
if directory:
source = os.path.join(directory, tail)
target_dir = os.path.join(directory, str(appid))
if os.path.isfile(source):
os.makedirs(target_dir, exist_ok=True)
try:
os.replace(source, os.path.join(target_dir, stored))
except OSError:
# Best effort. The row is still repointed below so the
# schema is consistent; a missing file is a re-upload.
pass
bind.execute(sa.text(
"UPDATE applications SET installpath = :path WHERE appid = :appid"
), {'path': f'{OLD_PREFIX}{appid}/{stored}', 'appid': appid})
def downgrade():
bind = op.get_bind()
rows = bind.execute(sa.text(
"SELECT appid, installpath FROM applications "
"WHERE installpath LIKE :prefix"
), {'prefix': OLD_PREFIX + '%'}).fetchall()
directory = _package_dir()
for appid, installpath in rows:
tail = installpath[len(OLD_PREFIX):]
if '/' not in tail:
continue
nested_id, _, stored = tail.partition('/')
if nested_id != str(appid):
continue
ext = os.path.splitext(stored)[1].lstrip('.').lower()
if not ext:
continue
flat = f'application-{appid}.{ext}'
if directory:
source = os.path.join(directory, str(appid), stored)
if os.path.isfile(source):
try:
os.replace(source, os.path.join(directory, flat))
# Remove the now-empty directory so a re-upgrade does not
# find a stale one. Only if empty: a directory with other
# files in it is not ours to remove.
os.rmdir(os.path.join(directory, str(appid)))
except OSError:
pass
bind.execute(sa.text(
"UPDATE applications SET installpath = :path WHERE appid = :appid"
), {'path': OLD_PREFIX + flat, 'appid': appid})

View File

@@ -30,10 +30,7 @@
<span v-if="app.isprinter" class="badge badge-lg badge-secondary">Printer App</span>
<span v-if="app.ishidden" class="badge badge-lg badge-dark">Hidden</span>
</div>
<div class="hero-links" v-if="app.applicationlink || app.installpath || app.documentationpath">
<a v-if="app.applicationlink" :href="fileHref(app.applicationlink)" target="_blank" class="hero-link">
<span class="link-icon">&#x1F517;</span> Launch Application
</a>
<div class="hero-links" v-if="app.installpath || app.documentationpath">
<a v-if="app.installpath" :href="fileHref(app.installpath)" target="_blank" class="hero-link">
<span class="link-icon">&#x2B07;</span> Download Files
</a>
@@ -61,6 +58,13 @@
<span v-else>{{ app.supportteamname || '-' }}</span>
</span>
</div>
<div class="info-row" v-if="app.applicationlink">
<span class="info-label">Application Link</span>
<!-- Reference only, deliberately not a link. It tells an
operator where to go for new versions; it is not a launch
or download action and must not read as one. -->
<span class="info-value mono reference-value">{{ app.applicationlink }}</span>
</div>
<div class="info-row" v-if="app.contacts && app.contacts.length">
<span class="info-label">Contacts</span>
<span class="info-value">
@@ -432,6 +436,11 @@ function kbKeywords(keywords) {
}
/* Support contacts stack one per line */
.reference-value {
word-break: break-all;
user-select: all;
}
.contact-line {
display: block;
}

View File

@@ -69,12 +69,51 @@
<input type="checkbox" v-model="form.isrequired" />
Required on all PCs
</label>
<label>
<input type="checkbox" v-model="form.ismonitorable" />
Monitorable
</label>
<label>
<input type="checkbox" v-model="form.ishidden" />
Hidden
</label>
</div>
<!-- The server rejects a monitorable app missing either field, so the
two inputs appear with the box rather than being hidden away in
another section where the rejection would read as a bug. -->
<div v-if="form.ismonitorable" class="monitor-fields">
<div class="form-group">
<label for="monitorprocessname">Process Name</label>
<input
id="monitorprocessname"
v-model="form.monitorprocessname"
type="text"
class="form-control"
placeholder="e.g. notepad.exe"
/>
<small class="form-hint">
The image name as it appears in Task Manager. This is what the
agent watches for.
</small>
</div>
<div class="form-group">
<label for="monitorlaunchpath">Launch Path</label>
<input
id="monitorlaunchpath"
v-model="form.monitorlaunchpath"
type="text"
class="form-control"
placeholder="e.g. C:\Program Files\Vendor\app.exe"
/>
<small class="form-hint">
Run on the endpoint when the process is found missing. A local
path on the PC, not a share.
</small>
</div>
</div>
<!-- Only shown when the user is newly ticking the box. An app that
was already required has an existing policy, so re-saving it is
not a new deployment and does not need re-acknowledging. -->
@@ -121,7 +160,7 @@
</div>
<div class="form-group">
<label for="installpath">Install Path</label>
<label for="installpath">Installer path</label>
<input
id="installpath"
v-model="form.installpath"
@@ -150,7 +189,29 @@
</div>
<small class="form-hint">
Up to 500MB. Anything larger belongs on the share - put its path in
Install Path instead. Downloads require a login.
Installer path instead. Downloads require a login.
</small>
</div>
<div class="form-group">
<label for="version">Version</label>
<input
id="version"
v-model="form.version"
type="text"
class="form-control"
placeholder="e.g. 2.4.1.0"
/>
<small class="form-hint">
<span v-if="detectedVersion">
Read from the installer. Change it if the vendor numbers releases
differently.
</span>
<span v-else>
Read from the installer where one is present. MSI and archive
files do not carry a version, so type it in to keep a later
re-import from looking like new software.
</span>
</small>
</div>
@@ -199,11 +260,26 @@
<div v-if="error" class="error-message">{{ error }}</div>
<div v-if="versionWarning" class="policy-warning">{{ versionWarning }}</div>
<div style="display: flex; gap: 0.5rem; margin-top: 1.5rem;">
<button type="submit" class="btn btn-primary" :disabled="saving">
{{ saving ? 'Saving...' : 'Save Application' }}
</button>
<router-link to="/applications" class="btn btn-secondary">Cancel</router-link>
<!-- Edit only: there is nothing to strip from an application that has
not been created yet. Pushed to the right and styled as a danger
action so it is not mistaken for Cancel. -->
<button
v-if="isEdit"
type="button"
class="btn btn-danger"
style="margin-left: auto;"
:disabled="busyRevert"
@click="revertToRecord"
>
{{ busyRevert ? 'Reverting...' : 'Revert to Record' }}
</button>
</div>
</form>
</div>
@@ -233,12 +309,19 @@ const form = ref({
islicenced: false,
isprinter: false,
isrequired: false,
ismonitorable: false,
monitorlaunchpath: '',
monitorprocessname: '',
ishidden: false,
applicationlink: '',
documentationpath: '',
installpath: '',
image: '',
applicationnotes: ''
applicationnotes: '',
// Not a column on applications - it creates/updates an AppVersion row on
// save. Held here so the field can sit next to the installer that produced
// it, which is where the operator is looking when it matters.
version: ''
})
const supportTeams = ref([])
@@ -261,6 +344,12 @@ const uploadedPackage = ref('')
const packageProgress = ref(null)
const busyImage = ref(false)
const busyPackage = ref(false)
const busyRevert = ref(false)
const detectedVersion = ref('')
const versionWarning = ref('')
// Versions already on the record, so saving an unchanged form does not try to
// re-create one and trip the duplicate check.
const existingVersions = ref([])
const PACKAGE_ACCEPT = '.exe,.msi,.msp,.zip,.7z,.cab,.iso,.appx,.msix,.ps1,.bat,.txt,.pdf'
const MAX_PACKAGE_BYTES = 500 * 1024 * 1024
@@ -313,6 +402,60 @@ async function removePackage() {
}
}
// Strips the application back to a bare catalogue record. Distinct from Delete
// on the list: the row, name and knowledgebase links survive, so this is the
// action for "stop deploying this" rather than "this never existed". Still
// irreversible, so it uses the same typed-OK gate the backend enforces.
async function revertToRecord() {
const answer = prompt(
'Remove all policy associations and uploaded executables? This action '
+ 'cannot be reversed. Type OK to proceed')
if ((answer || '').trim().toUpperCase() !== 'OK') return
busyRevert.value = true
error.value = ''
try {
const response = await applicationsApi.revertToRecord(
route.params.id, (answer || '').trim())
const app = response.data.data
// Re-seed from the server rather than guessing which fields it cleared.
// The image is deliberately untouched: revert removes executables and
// policy associations, not the record's icon.
form.value.installpath = app.installpath || ''
form.value.isrequired = app.isrequired
// Revert drops every AppVersion row, so the field must not keep showing a
// version that no longer exists.
form.value.version = ''
uploadedPackage.value = ''
pendingPackage.value = null
detectedVersion.value = ''
versionWarning.value = ''
} catch (err) {
error.value = apiError(err, 'Failed to revert the application')
} finally {
busyRevert.value = false
}
}
// Records the typed version against the application, if it is new. Creating a
// version that already exists is not an error worth failing the save over -
// re-saving a form without touching the field is the normal case, and the
// backend answers 409, which would otherwise surface as "failed to save" on a
// save that in fact succeeded.
async function saveVersion(appid) {
const version = (form.value.version || '').trim()
if (!version) return
if (existingVersions.value.some(v => v.version === version)) return
try {
await applicationsApi.createVersion(appid, { version })
existingVersions.value = [{ version }, ...existingVersions.value]
} catch (err) {
if (err.response?.status === 409) return
throw err
}
}
// The image field holds either a served URL (new) or a bare filename from the
// classic site, which is still rendered out of /images/applications/.
function imageSrcFor(value) {
@@ -341,13 +484,22 @@ onMounted(async () => {
islicenced: app.islicenced || false,
isprinter: app.isprinter || false,
isrequired: app.isrequired || false,
ismonitorable: app.ismonitorable || false,
monitorlaunchpath: app.monitorlaunchpath || '',
monitorprocessname: app.monitorprocessname || '',
ishidden: app.ishidden || false,
applicationlink: app.applicationlink || '',
documentationpath: app.documentationpath || '',
installpath: app.installpath || '',
image: app.image || '',
applicationnotes: app.applicationnotes || ''
applicationnotes: app.applicationnotes || '',
// The detail payload already carries versions newest-first, so the
// current one comes free rather than costing another request.
version: (app.versions && app.versions.length)
? (app.versions[0].version || '')
: ''
}
existingVersions.value = app.versions || []
imagePreview.value = imageSrcFor(app.image)
wasRequired.value = app.isrequired || false
if ((app.installpath || '').startsWith('/api/applications/package/')) {
@@ -371,6 +523,20 @@ async function saveApplication() {
return
}
// Mirrors _validate_monitorable on the server. Checked here too so the
// operator is told which field is missing while they are still looking at it,
// rather than after a round trip.
if (form.value.ismonitorable) {
if (!(form.value.monitorprocessname || '').trim()) {
error.value = 'A monitorable application needs a process name.'
return
}
if (!(form.value.monitorlaunchpath || '').trim()) {
error.value = 'A monitorable application needs a launch path.'
return
}
}
saving.value = true
try {
@@ -382,6 +548,15 @@ async function saveApplication() {
islicenced: form.value.islicenced,
isprinter: form.value.isprinter,
isrequired: form.value.isrequired,
ismonitorable: form.value.ismonitorable,
// Cleared when the box is unticked so a later re-tick cannot silently
// revive a stale process name the operator can no longer see.
monitorlaunchpath: form.value.ismonitorable
? (form.value.monitorlaunchpath || null)
: null,
monitorprocessname: form.value.ismonitorable
? (form.value.monitorprocessname || null)
: null,
ishidden: form.value.ishidden,
applicationlink: form.value.applicationlink || null,
documentationpath: form.value.documentationpath || null,
@@ -408,12 +583,33 @@ async function saveApplication() {
}
if (pendingPackage.value) {
packageProgress.value = 0
await applicationsApi.uploadPackage(appid, pendingPackage.value,
percent => { packageProgress.value = percent })
const uploaded = await applicationsApi.uploadPackage(
appid, pendingPackage.value,
percent => { packageProgress.value = percent })
pendingPackage.value = null
packageProgress.value = null
// An installer with no readable version resource is how duplicate
// version rows got created: the operator saved, saw no version, and
// imported the same file again. MSIs and archives are not PE files and
// never carry one, so ask rather than guess.
detectedVersion.value = uploaded.data.data.detectedversion || ''
if (detectedVersion.value && !form.value.version.trim()) {
form.value.version = detectedVersion.value
}
if (!form.value.version.trim()) {
versionWarning.value =
`No version could be read from ${uploaded.data.data.uploadedfilename}. `
+ 'Enter one in the Version field above so a later import is not '
+ 'mistaken for new software.'
// The application and installer are already saved; only the version is
// outstanding, so stay on the form rather than losing that work.
return
}
}
await saveVersion(appid)
router.push('/applications')
} catch (err) {
console.error('Error saving application:', err)
@@ -489,11 +685,14 @@ async function saveApplication() {
.policy-warning {
margin-top: 1rem;
padding: 0.75rem 1rem;
border: 1px solid var(--warning, #b45309);
border: 1px solid var(--warning);
border-left-width: 4px;
border-radius: 4px;
background: var(--warning-bg, #fffbeb);
color: var(--text, #333);
/* Derived from the real token rather than a fixed tint: a hardcoded light
value here survives the theme switch and renders light-on-light in dark
mode. --warning-bg does not exist in style.css. */
background: color-mix(in srgb, var(--warning) 12%, var(--bg-card-solid));
color: var(--text);
}
.policy-warning p {
@@ -508,4 +707,13 @@ async function saveApplication() {
cursor: pointer;
font-weight: 600;
}
.monitor-fields {
margin-top: 1rem;
padding: 0.75rem 1rem;
border: 1px solid var(--border);
border-left: 4px solid var(--warning);
border-radius: 4px;
background: color-mix(in srgb, var(--warning) 6%, var(--bg-card-solid));
}
</style>

View File

@@ -44,9 +44,6 @@
<a v-if="app.installpath" :href="fileHref(app.installpath)" target="_blank" title="Download Installation Files" class="icon-download">
&#x2B07;
</a>
<a v-else-if="app.applicationlink" :href="fileHref(app.applicationlink)" target="_blank" title="Application Link" class="icon-link">
&#x1F517;
</a>
</td>
<td class="icon-cell">
<a v-if="app.documentationpath" :href="fileHref(app.documentationpath)" target="_blank" title="View Documentation" class="icon-docs">
@@ -178,19 +175,23 @@ function debouncedSearch() {
}, 300)
}
// The server deactivates rather than drops the row, so this is reversible and
// the prompt does not claim otherwise. It does warn about the fleet-wide
// consequence, which is the part that is not obvious from a Delete button.
// This permanently drops the row, its versions and its uploaded installer, so
// the prompt says so plainly and requires the word OK to be typed. A plain
// confirm() is one misplaced click away from destroying licensed software.
async function deleteApplication(app) {
const warning = app.isrequired
? `\n\n"${app.appname}" is currently required on all PCs. Removing it also `
+ 'withdraws the policy that installs it.'
: ''
if (!confirm(`Remove "${app.appname}" from the catalogue?${warning}`)) return
const answer = prompt(
`Permanently delete "${app.appname}", its versions and its uploaded `
+ `executables? This action cannot be reversed.${warning}\n\n`
+ 'Type OK to proceed')
if ((answer || '').trim().toUpperCase() !== 'OK') return
try {
await applicationsApi.delete(app.appid)
await applicationsApi.delete(app.appid, (answer || '').trim())
loadApplications()
} catch (error) {
console.error('Error deleting application:', error)
@@ -227,10 +228,6 @@ function changePerPage(newPerPage) {
color: var(--success);
}
.icon-link {
color: var(--link);
}
.icon-docs {
color: var(--secondary);
}

View File

@@ -215,7 +215,8 @@ def list_endpoints():
query = (db.session.query(EndpointState, Computer.computerid,
Computer.computertypeid, ComputerType.computertype,
Asset.assetnumber, statusrank.label('statusrank'))
Asset.assetnumber, Asset.name,
statusrank.label('statusrank'))
.join(Asset, Asset.assetid == EndpointState.assetid)
.join(Computer, Computer.assetid == EndpointState.assetid)
.outerjoin(ComputerType,
@@ -227,7 +228,8 @@ def list_endpoints():
if search := (request.args.get('search') or '').strip():
like = f'%{search}%'
machinehits = _assetids_serving_machine_like(like)
clauses = [EndpointState.hostname.ilike(like), Asset.assetnumber.ilike(like)]
clauses = [EndpointState.hostname.ilike(like), Asset.assetnumber.ilike(like),
Asset.name.ilike(like)]
if machinehits:
clauses.append(EndpointState.assetid.in_(machinehits))
query = query.filter(db.or_(*clauses))
@@ -265,10 +267,14 @@ def list_endpoints():
health = _health_for_assets(states, computertypes)
data = []
for state, computerid, computertypeid, computertype, assetnumber, rank in items:
for (state, computerid, computertypeid, computertype, assetnumber, alias,
rank) in items:
row = state.to_dict()
row['computerid'] = computerid
row['assetnumber'] = assetnumber
# Named 'alias' rather than 'name' because the fleet row already
# carries a hostname; 'name' next to 'hostname' reads as a duplicate.
row['alias'] = alias
row['computertypeid'] = computertypeid
row['computertype'] = computertype
row['uptimedisplay'] = _format_uptime(state.uptimeseconds)

View File

@@ -87,6 +87,7 @@
<input type="checkbox" :checked="allselected" @change="toggleAll" title="Select all on this page" />
</th>
<th>Status</th>
<th>Alias</th>
<th>Hostname</th>
<th>Machine</th>
<th>PC Type</th>
@@ -111,6 +112,7 @@
<td>
<span class="status-dot" :class="item.status"></span>{{ statusLabel(item.status) }}
</td>
<td>{{ item.alias || '-' }}</td>
<td>{{ item.hostname || '-' }}</td>
<td>
<!-- What stops if this PC is dead. On a shop floor that is
@@ -130,7 +132,7 @@
<td>{{ formatRelative(item.lastreportedat) }}</td>
</tr>
<tr v-if="endpoints.length === 0">
<td :colspan="cancommand ? 10 : 9" style="text-align: center; color: var(--text-light);">
<td :colspan="cancommand ? 11 : 10" style="text-align: center; color: var(--text-light);">
No endpoints found
</td>
</tr>

View File

@@ -4,6 +4,7 @@ import glob
import hashlib
import re
import os
import shutil
from flask import Blueprint, request, current_app
from flask_jwt_extended import jwt_required
@@ -63,6 +64,28 @@ POLICY_ACK_MESSAGE = (
'Confirm to create the policy.'
)
# Deleting an application is irreversible and takes its versions and uploaded
# installer with it, so the caller has to type this rather than click once.
# Checked server-side as well as in the dialog: the endpoint is reachable
# directly, and a confirmation that only exists in the UI is not a safeguard.
DELETE_CONFIRM_TOKEN = 'OK'
DELETE_CONFIRM_MESSAGE = (
'Deleting an application permanently removes it, its versions, and its '
'uploaded installer. This cannot be undone. Type OK to confirm.'
)
def _delete_confirmed(data):
"""True when the caller typed the confirmation token exactly.
Compared case-sensitively after stripping surrounding whitespace. Accepting
"ok" would defeat the point of asking for deliberate input.
"""
if not isinstance(data, dict):
return False
return str(data.get('confirm', '')).strip() == DELETE_CONFIRM_TOKEN
def _validate_monitorable(data, existing=None):
"""Return an error message if monitorable is set without its two fields.
@@ -122,6 +145,11 @@ def _sync_required_app_policy(app):
# msi/exe this is for, small enough that the disk cannot vanish behind it.
MAX_PACKAGE_BYTES = 500 * 1024 * 1024
# How much of an installer to read when looking for its version resource. The
# PE resource directory is usually within the first few MB; reading the whole
# file would mean holding a 500MB upload in memory to learn a version string.
PE_VERSION_SCAN_BYTES = 32 * 1024 * 1024
def _appimage_dir():
return os.path.join(current_app.instance_path, 'applicationimages')
@@ -145,6 +173,46 @@ def _sha256_of(stream):
return digest.hexdigest()
def _detect_installer_version(stream):
"""Read the FileVersion out of an uploaded PE installer, or None.
Deliberately reuses the endpoints plugin's PE reader rather than parsing the
format again here. A version detected at upload has to match what the agent
reports from the installed machine, and two separate implementations of the
same structure walk would drift apart and make an endpoint look
out-of-compliance for no real reason.
FileVersion only, never ProductVersion: ProductVersion may carry semver
metadata (1.5.0+abc123) which is not a version number we can compare or use
in a path. Returns None for MSI and archives, which are not PE files at all
and simply have no version to read.
"""
try:
from plugins.endpoints.api.agentstore import read_pe_file_version
except ImportError:
return None
position = stream.tell()
try:
# The version resource lives in the PE header near the start, but the
# resource directory it points into can sit much deeper. Read a bounded
# prefix so a 500MB installer is not pulled into memory whole.
data = stream.read(PE_VERSION_SCAN_BYTES)
finally:
stream.seek(position)
# The parser documents itself as never raising, but it walks offsets read
# out of an attacker-supplied file. A version is a convenience; it must not
# cost the operator the upload of a 500MB installer if that guarantee ever
# slips.
try:
return read_pe_file_version(data)
except Exception:
current_app.logger.warning('Version detection failed for an installer',
exc_info=True)
return None
def _replace_upload(directory, stem, ext, upload):
"""Save one file per application, replacing any prior extension.
@@ -400,31 +468,178 @@ def update_application(app_id: int):
return success_response(payload, message='Application updated')
def _remove_package_files(app_id: int):
"""Delete the stored installer(s) for an application, leaving the image.
Split out from _remove_upload_files because revert-to-record removes the
executables but keeps the icon: the record stays in the catalogue and a
record with no image is a worse record. Delete calls both.
Best-effort by design: a missing or unreadable file must not abort the
operation and strand the row. Returns the filenames actually removed so the
audit entry records what went.
"""
removed = []
packagedir = os.path.join(_apppackage_dir(), str(app_id))
for path in glob.glob(os.path.join(packagedir, '*')):
removed.append(os.path.basename(path))
if os.path.isdir(packagedir):
shutil.rmtree(packagedir, ignore_errors=True)
if os.path.isdir(packagedir):
current_app.logger.warning(
'Could not remove %s for appid=%s', packagedir, app_id)
return removed
def _remove_upload_files(app_id: int):
"""Delete the stored installer and image for an application.
Used by delete, which takes everything. Revert calls
_remove_package_files() instead so the image survives.
"""
removed = []
# The image is still one flat file per application; the installer now lives
# in a per-application directory so it can keep its vendor filename.
pattern = os.path.join(_appimage_dir(),
secure_filename(f'application-{app_id}') + '.*')
for path in glob.glob(pattern):
try:
os.remove(path)
removed.append(os.path.basename(path))
except OSError:
current_app.logger.warning(
'Could not remove %s while deleting appid=%s', path, app_id)
return removed + _remove_package_files(app_id)
def _clear_application_references(app_id: int):
"""Detach rows that point at this application before it is deleted.
The FKs differ in intent, so they are not treated the same:
- appversions.appid is NOT NULL and the versions are meaningless without
the application, so they go.
- computerinstalledapps.appid is NOT NULL; these are inventory links, so
the link goes while the computer stays.
- knowledgebase.appid is nullable and the article is somebody's writing,
so it is unlinked rather than deleted.
"""
AppVersion.query.filter_by(appid=app_id).delete(synchronize_session=False)
models = _computer_models()
if models is not None:
_, ComputerInstalledApp = models
ComputerInstalledApp.query.filter_by(appid=app_id).delete(
synchronize_session=False)
try:
from plugins.knowledgebase.models import KnowledgeBase
except ImportError:
return
KnowledgeBase.query.filter_by(appid=app_id).update(
{'appid': None}, synchronize_session=False)
@applications_bp.route('/<int:app_id>', methods=['DELETE'])
@require_permission('applications.delete')
def delete_application(app_id: int):
"""Delete (deactivate) an application."""
"""Permanently delete an application, its versions, and its uploads.
This is a hard delete. The previous soft delete only set isactive=False,
which left the name reserved forever: creating an application with the same
name failed with "already exists" against a row no screen would show.
"""
app = db.session.get(Application, app_id)
if not app:
return error_response(ErrorCodes.NOT_FOUND, 'Application not found', http_code=404)
app.isactive = False
if not _delete_confirmed(request.get_json(silent=True)):
return error_response(ErrorCodes.VALIDATION_ERROR, DELETE_CONFIRM_MESSAGE)
# Withdraw the generated policy too. Deactivating an application while its
# policy still runs would keep installing something the catalogue says is
# gone, and nothing in the UI would explain why. A customised policy is
# left alone, same as unticking the box.
# Withdraw the generated policy first. A policy that outlived its
# application would keep installing something that no longer exists.
appname = app.appname
app.isrequired = False
_sync_required_app_policy(app)
AuditLog.log('deleted', 'Application', entityid=app.appid, entityname=app.appname)
_clear_application_references(app_id)
removed = _remove_upload_files(app_id)
db.session.delete(app)
# The audit row is the only record left that this application existed, so
# it names the files that went with it.
AuditLog.log('deleted', 'Application', entityid=app_id, entityname=appname,
changes={'removedfiles': removed} if removed else None)
db.session.commit()
return success_response(message='Application deleted')
REVERT_CONFIRM_MESSAGE = (
'Remove all policy associations and uploaded executables? This action '
"cannot be reversed. Type OK to proceed"
)
@applications_bp.route('/<int:app_id>/revert', methods=['POST'])
@require_permission('applications.edit')
def revert_application_to_record(app_id: int):
"""Strip an application back to a catalogue record.
The middle ground between editing and deleting. Deleting loses the name,
the description and the knowledgebase links, which is too much when the
intent is only to stop deploying something and re-upload a clean installer.
This withdraws the generated policy, drops the version rows and removes the
uploaded files, while the application itself stays in the catalogue.
Inventory links are deliberately kept: computerinstalledapps records that a
machine really does have this software on it, and that stays true whether
or not we still ship an installer for it. Deleting those would make the
fleet look clean when it is not.
Gated on applications.edit rather than applications.delete: the record
survives, and a user trusted to change an application is trusted to stop
deploying it.
"""
app = db.session.get(Application, app_id)
if not app:
return error_response(ErrorCodes.NOT_FOUND, 'Application not found',
http_code=404)
if not _delete_confirmed(request.get_json(silent=True)):
return error_response(ErrorCodes.VALIDATION_ERROR, REVERT_CONFIRM_MESSAGE)
# Withdraw the generated policy first, for the same reason delete does: a
# policy left behind would keep pushing an installer we just removed.
app.isrequired = False
_sync_required_app_policy(app)
AppVersion.query.filter_by(appid=app_id).delete(synchronize_session=False)
# Executables only. The image is not an executable and the warning does not
# offer to remove it - the record survives this operation, and stripping its
# icon would degrade the thing we are explicitly keeping.
removed = _remove_package_files(app_id)
# Only clear an installpath we own. A UNC path typed by an operator points
# at their share, not our upload directory, and is not ours to wipe.
if (app.installpath or '').startswith(APP_PACKAGE_URL_PREFIX):
app.installpath = None
app.installersha256 = None
AuditLog.log('updated', 'Application', entityid=app_id,
entityname=app.appname,
changes={'revertedtorecord': {'new': True},
'removedfiles': removed})
db.session.commit()
return success_response(app.to_dict(), message='Application reverted to record')
# ---- Versions ----
@applications_bp.route('/<int:app_id>/versions', methods=['GET'])
@@ -736,14 +951,43 @@ def delete_application_image(app_id: int):
return success_response(app.to_dict(), message='Application image removed')
def _store_package(app_id: int, upload):
"""Save an installer under its own vendor filename, one per application.
Keeping the shipped name matters at import: "DeviceLoggerSetup.msi" is
recognisable as the thing already in the catalogue, whereas the old
application-<id>.msi told the operator nothing and made a re-import look
like new software, which is how duplicate applications and versions got
created in the first place.
Files are namespaced by application id so two applications shipping a
generic "setup.exe" cannot overwrite each other. Any previous installer for
this application is removed first, so replacing one does not accumulate.
Returns (storedname, originalname).
"""
directory = os.path.join(_apppackage_dir(), str(app_id))
if os.path.isdir(directory):
shutil.rmtree(directory, ignore_errors=True)
os.makedirs(directory, exist_ok=True)
original = os.path.basename(upload.filename or '')
# secure_filename can reduce a name to nothing (all-unicode, all-dots), and
# an empty name would save as a bare directory entry.
stored = secure_filename(original) or 'installer{}'.format(
os.path.splitext(original)[1].lower())
upload.save(os.path.join(directory, stored))
return stored, original
@applications_bp.route('/<int:app_id>/package', methods=['POST'])
@require_permission('applications.edit')
def upload_application_package(app_id: int):
"""Upload (or replace) the installer for an application.
multipart/form-data: file=<installer>. The original filename is kept in
installpath so the download arrives named the way the vendor shipped it,
and the stored copy is application-<id><ext>.
multipart/form-data: file=<installer>. The installer is stored under the
filename the vendor shipped, inside a per-application directory.
"""
app = db.session.get(Application, app_id)
if not app:
@@ -773,8 +1017,11 @@ def upload_application_package(app_id: int):
'on the share and link to them with Install Path.'.format(
size / 1048576, MAX_PACKAGE_BYTES / 1048576))
filename = _replace_upload(_apppackage_dir(), f'application-{app_id}', ext, upload)
app.installpath = '{}{}'.format(APP_PACKAGE_URL_PREFIX, filename)
# Read the version before saving: the stream is consumed by save().
detectedversion = _detect_installer_version(upload.stream)
filename, _original = _store_package(app_id, upload)
app.installpath = '{}{}/{}'.format(APP_PACKAGE_URL_PREFIX, app_id, filename)
# The endpoint installer push pins bytes by digest; compute it once here so
# the server, not the agent, decides what the correct payload is. Stamp it
@@ -805,6 +1052,10 @@ def upload_application_package(app_id: int):
return success_response(
{**app.to_dict(), 'uploadedfilename': upload.filename, 'bytes': size,
'sha256': sha256,
# None means "no version resource" - the form must ask the operator
# rather than silently saving an installer with no version, which is
# how duplicate version rows appeared.
'detectedversion': detectedversion,
'appversionid': stamped_version.appversionid if stamped_version else None},
message='Installer uploaded')
@@ -821,6 +1072,10 @@ def serve_application_package(filename):
Gated on applications.download, not applications.view: a guest may be
granted the metadata permission, and "an open URL would publish it" is just
as true of a URL any anonymous visitor can reach.
filename is "<appid>/<vendorname>" now that installers keep the name they
shipped under. send_from_directory refuses to escape the base directory, so
the appid segment cannot be walked out of with "..".
"""
return send_upload(_apppackage_dir(), filename, as_attachment=True)
@@ -835,12 +1090,12 @@ def delete_application_package(app_id: int):
f'Application with ID {app_id} not found',
http_code=404)
for old in glob.glob(os.path.join(_apppackage_dir(),
secure_filename(f'application-{app_id}') + '.*')):
os.remove(old)
shutil.rmtree(os.path.join(_apppackage_dir(), str(app_id)),
ignore_errors=True)
# Only clear the path if it pointed at the upload; a site's own share path
# was typed by a person and is not ours to wipe.
if (app.installpath or '').startswith(APP_PACKAGE_URL_PREFIX):
app.installpath = None
app.installersha256 = None
db.session.commit()
return success_response(app.to_dict(), message='Installer removed')

View File

@@ -1,98 +0,0 @@
"""Tests for deactivating an application and what happens to its policy.
Delete here is a soft delete: the row is deactivated, not dropped. That is easy
to forget, because the generated install policy is a separate row that does NOT
follow automatically. Left alone it would keep installing an application the
catalogue reports as gone, and nothing on screen would explain why.
So the deactivation withdraws the policy as well, on the same ownership terms as
unticking the box: ours to remove, theirs to keep.
"""
from shopdb.core.models import Application
from plugins.computers.models import (ConfigurationPolicy, POLICY_SOURCE_LOCAL,
POLICY_SOURCE_AUTOAPP)
from plugins.computers.services.apppolicy import auto_policy_name
def _required_app(client, auth_headers, name='Fleet App'):
response = client.post('/api/applications',
json={'appname': name, 'isrequired': True,
'acknowledgepolicy': True},
headers=auth_headers)
assert response.status_code == 201, response.get_json()
return Application.query.filter_by(appname=name).first()
def test_deleting_an_application_deactivates_rather_than_drops_it(
client, db, auth_headers):
app = Application(appname='Plain App')
db.session.add(app)
db.session.commit()
appid = app.appid
response = client.delete(f'/api/applications/{appid}', headers=auth_headers)
assert response.status_code == 200, response.get_json()
assert db.session.get(Application, appid).isactive is False
def test_deleting_a_required_application_withdraws_its_policy(
client, db, auth_headers):
app = _required_app(client, auth_headers)
assert ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first() is not None
response = client.delete(f'/api/applications/{app.appid}',
headers=auth_headers)
assert response.status_code == 200, response.get_json()
# The policy must not outlive the application it installs.
assert ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first() is None
def test_deleting_an_application_keeps_a_customised_policy(
client, db, auth_headers):
# Same rule as unticking the box: once an admin has edited the policy it is
# theirs, and a delete on another screen must not discard their work.
app = _required_app(client, auth_headers)
policy = ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first()
policy.source = POLICY_SOURCE_LOCAL
db.session.commit()
response = client.delete(f'/api/applications/{app.appid}',
headers=auth_headers)
assert response.status_code == 200, response.get_json()
survivor = ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first()
assert survivor is not None
assert survivor.source == POLICY_SOURCE_LOCAL
def test_deleting_clears_the_required_flag(client, db, auth_headers):
# Otherwise reactivating the application later would show it as required
# while no policy enforces it.
app = _required_app(client, auth_headers)
client.delete(f'/api/applications/{app.appid}', headers=auth_headers)
assert db.session.get(Application, app.appid).isrequired is False
def test_delete_requires_the_delete_permission(client, db, auth_headers,
member_headers):
# Authenticated but unprivileged. The button is hidden for this user in the
# UI; this pins that the endpoint refuses them regardless.
app = Application(appname='Plain App')
db.session.add(app)
db.session.commit()
response = client.delete(f'/api/applications/{app.appid}',
headers=member_headers)
assert response.status_code in (401, 403), response.get_json()
assert db.session.get(Application, app.appid).isactive is not False

View File

@@ -0,0 +1,139 @@
"""Tests for permanently deleting an application.
This used to be a soft delete, and that was the bug: setting isactive=False
left the row in the table, so the name stayed reserved forever. Creating an
application with the same name failed with "already exists" pointing at a row
no screen would show. Nothing in the UI could explain or clear it.
So delete is now a real delete, and these tests pin the parts that are easy to
get wrong: the dependent rows that would otherwise orphan or block the delete,
the uploaded files that would otherwise linger on disk, and the typed
confirmation that stops it happening on a stray click.
"""
import os
from shopdb.core.models import Application, AppVersion
from plugins.computers.models import ConfigurationPolicy
from plugins.computers.services.apppolicy import auto_policy_name
def _app(db, name='Test App', **kwargs):
app = Application(appname=name, **kwargs)
db.session.add(app)
db.session.commit()
return app
def _delete(client, headers, appid, confirm='OK'):
return client.delete(f'/api/applications/{appid}',
json={'confirm': confirm} if confirm is not None else {},
headers=headers)
def test_delete_removes_the_row_entirely(client, db, auth_headers):
app = _app(db)
appid = app.appid
response = _delete(client, auth_headers, appid)
assert response.status_code == 200, response.get_json()
assert db.session.get(Application, appid) is None
def test_the_name_is_reusable_after_a_delete(client, db, auth_headers):
# The bug this change exists to fix. A soft-deleted row kept its name
# reserved and the conflict named an application the user could not see.
app = _app(db, name='DeviceLogger')
_delete(client, auth_headers, app.appid)
response = client.post('/api/applications',
json={'appname': 'DeviceLogger'},
headers=auth_headers)
assert response.status_code == 201, response.get_json()
def test_delete_is_refused_without_the_typed_confirmation(client, db,
auth_headers):
app = _app(db)
response = _delete(client, auth_headers, app.appid, confirm=None)
assert response.status_code == 400, response.get_json()
assert db.session.get(Application, app.appid) is not None
def test_delete_is_refused_when_the_confirmation_is_wrong_case(client, db,
auth_headers):
# Accepting "ok" would defeat the point of asking for deliberate input.
app = _app(db)
response = _delete(client, auth_headers, app.appid, confirm='ok')
assert response.status_code == 400, response.get_json()
assert db.session.get(Application, app.appid) is not None
def test_delete_takes_the_versions_with_it(client, db, auth_headers):
# appversions.appid is NOT NULL, so leaving these behind would either
# orphan them or block the delete outright.
app = _app(db)
db.session.add(AppVersion(appid=app.appid, version='1.0.0.0'))
db.session.commit()
appid = app.appid
response = _delete(client, auth_headers, appid)
assert response.status_code == 200, response.get_json()
assert AppVersion.query.filter_by(appid=appid).count() == 0
def test_delete_withdraws_the_generated_policy(client, db, auth_headers):
response = client.post('/api/applications',
json={'appname': 'Fleet App', 'isrequired': True,
'acknowledgepolicy': True},
headers=auth_headers)
assert response.status_code == 201, response.get_json()
app = Application.query.filter_by(appname='Fleet App').first()
assert ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first() is not None
_delete(client, auth_headers, app.appid)
assert ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first() is None
def test_delete_removes_the_uploaded_installer(client, db, auth_headers):
app = _app(db)
upload = client.post(f'/api/applications/{app.appid}/package',
data={'file': (__import__('io').BytesIO(b'MZfake'),
'setup.exe')},
content_type='multipart/form-data',
headers=auth_headers)
assert upload.status_code == 200, upload.get_json()
stored = db.session.get(Application, app.appid).installpath
assert stored
from shopdb.core.api.applications import _apppackage_dir
packagedir = os.path.join(_apppackage_dir(), str(app.appid))
# Guard the guard: if the upload did not land where we think, the
# post-delete assertion below would pass without proving anything.
assert os.listdir(packagedir) == ['setup.exe']
_delete(client, auth_headers, app.appid)
# Nothing may remain on disk for this application.
assert not os.path.isdir(packagedir)
def test_delete_requires_the_delete_permission(client, db, member_headers):
app = _app(db)
response = _delete(client, member_headers, app.appid)
assert response.status_code in (401, 403), response.get_json()
assert db.session.get(Application, app.appid) is not None

View File

@@ -0,0 +1,142 @@
"""Tests for the monitorable flag and the two fields it depends on.
A monitorable application tells the agent to watch for a process and relaunch it
when it goes missing. That needs an image name to watch for and a path to run,
so the flag is meaningless without both - a monitor with no process name has
nothing to check, and one with no launch path has nothing to do about it.
The rule lives in _validate_monitorable rather than in a DB constraint so the
columns stay nullable for rows that predate the feature. That makes it a rule
code has to keep choosing to apply, which is what these tests pin: it must hold
on create AND on update, including the update that only ticks the box and sends
nothing else.
"""
from shopdb.core.models import Application
def _app(db, name='Test App', **kwargs):
app = Application(appname=name, **kwargs)
db.session.add(app)
db.session.commit()
return app
def _create(client, headers, **fields):
payload = {'appname': 'Monitored App'}
payload.update(fields)
return client.post('/api/applications', json=payload, headers=headers)
def _update(client, headers, appid, **fields):
return client.put(f'/api/applications/{appid}', json=fields,
headers=headers)
def _message(response):
"""The error text, which sits at data.error.message rather than top level."""
return response.get_json()['data']['error']['message'].lower()
def test_creating_a_monitorable_app_needs_a_process_name(client, auth_headers):
response = _create(client, auth_headers, ismonitorable=True,
monitorlaunchpath=r'C:\Vendor\app.exe')
assert response.status_code == 400
assert 'process name' in _message(response)
def test_creating_a_monitorable_app_needs_a_launch_path(client, auth_headers):
response = _create(client, auth_headers, ismonitorable=True,
monitorprocessname='app.exe')
assert response.status_code == 400
assert 'launch path' in _message(response)
def test_a_monitorable_app_with_both_fields_saves(client, auth_headers, db):
response = _create(client, auth_headers, ismonitorable=True,
monitorprocessname='app.exe',
monitorlaunchpath=r'C:\Vendor\app.exe')
assert response.status_code == 201, response.get_json()
app = Application.query.filter_by(appname='Monitored App').first()
assert app.ismonitorable is True
assert app.monitorprocessname == 'app.exe'
def test_a_non_monitorable_app_needs_neither_field(client, auth_headers):
# The rule must not leak onto ordinary applications, which are the majority.
response = _create(client, auth_headers, ismonitorable=False)
assert response.status_code == 201, response.get_json()
def test_ticking_the_box_alone_is_refused(client, auth_headers, db):
"""The case a payload-only check would miss.
The form sends just the changed field, so an update that ticks monitorable
on a record with no launch path or process name arrives carrying nothing
else. Validating the payload in isolation would see no conflict and let it
through, storing a monitor that cannot run.
"""
app = _app(db, name='Plain App')
response = _update(client, auth_headers, app.appid, ismonitorable=True)
assert response.status_code == 400
# Launch path is checked first, so that is the field named. What matters is
# that the merged state was rejected at all, not which half it complained
# about.
assert 'launch path' in _message(response)
def test_ticking_the_box_is_allowed_when_the_row_already_has_the_fields(
client, auth_headers, db):
# The mirror of the above: validation reads the MERGED result, so stored
# values satisfy it and the operator is not asked to retype them.
app = _app(db, name='Ready App', monitorprocessname='app.exe',
monitorlaunchpath=r'C:\Vendor\app.exe')
response = _update(client, auth_headers, app.appid, ismonitorable=True)
assert response.status_code == 200, response.get_json()
assert Application.query.get(app.appid).ismonitorable is True
def test_clearing_a_field_on_a_monitorable_app_is_refused(client, auth_headers,
db):
# Blanking the process name is the same broken state as never setting it,
# so it has to be refused from this direction too.
app = _app(db, name='Live Monitor', ismonitorable=True,
monitorprocessname='app.exe',
monitorlaunchpath=r'C:\Vendor\app.exe')
response = _update(client, auth_headers, app.appid,
monitorprocessname='')
assert response.status_code == 400
assert 'process name' in _message(response)
def test_whitespace_does_not_count_as_a_process_name(client, auth_headers):
response = _create(client, auth_headers, ismonitorable=True,
monitorprocessname=' ',
monitorlaunchpath=r'C:\Vendor\app.exe')
assert response.status_code == 400
assert 'process name' in _message(response)
def test_unticking_the_box_releases_the_requirement(client, auth_headers, db):
# Turning monitoring off must not be blocked by the fields it needed while
# it was on, or an operator cannot undo a mistake without a round trip.
app = _app(db, name='Live Monitor', ismonitorable=True,
monitorprocessname='app.exe',
monitorlaunchpath=r'C:\Vendor\app.exe')
response = _update(client, auth_headers, app.appid, ismonitorable=False,
monitorprocessname='', monitorlaunchpath='')
assert response.status_code == 200, response.get_json()
assert Application.query.get(app.appid).ismonitorable is False

View File

@@ -0,0 +1,196 @@
"""Tests for reverting an application back to a plain catalogue record.
Revert is the middle ground between editing and deleting: it withdraws the
generated policy, drops the version rows and removes the uploaded executables,
while the application itself stays in the catalogue.
The line these tests defend is what revert must NOT touch. The warning offers
to remove "policy associations and uploaded executables" and nothing else, so
the image, the name, the description and the knowledgebase links all have to
survive. An operator who reverts to clean up an installer should not discover
afterwards that the record lost its icon.
"""
import io
import os
from shopdb.core.models import Application, AppVersion
from plugins.computers.models import ConfigurationPolicy
from plugins.computers.services.apppolicy import auto_policy_name
def _app(db, name='Test App', **kwargs):
app = Application(appname=name, **kwargs)
db.session.add(app)
db.session.commit()
return app
def _revert(client, headers, appid, confirm='OK'):
return client.post(f'/api/applications/{appid}/revert',
json={'confirm': confirm} if confirm is not None else {},
headers=headers)
def _upload_package(client, headers, appid, name='setup.exe', data=b'MZfake'):
return client.post(f'/api/applications/{appid}/package',
data={'file': (io.BytesIO(data), name)},
content_type='multipart/form-data',
headers=headers)
def _upload_image(client, headers, appid):
# A one-pixel PNG; the route only needs a plausible image file.
png = (b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00'
b'\x01\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc'
b'\x00\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`'
b'\x82')
return client.post(f'/api/applications/{appid}/image',
data={'file': (io.BytesIO(png), 'icon.png')},
content_type='multipart/form-data',
headers=headers)
def test_revert_keeps_the_application_row(client, db, auth_headers):
# The whole point of revert over delete: the record survives.
app = _app(db, name='DeviceLogger')
appid = app.appid
response = _revert(client, auth_headers, appid)
assert response.status_code == 200, response.get_json()
survivor = db.session.get(Application, appid)
assert survivor is not None
assert survivor.appname == 'DeviceLogger'
def test_revert_keeps_the_image(client, db, auth_headers):
# The warning offers to remove policy associations and executables. It says
# nothing about the icon, and a record kept on purpose should keep it.
app = _app(db)
appid = app.appid
assert _upload_image(client, auth_headers, appid).status_code == 200
stored = db.session.get(Application, appid).image
assert stored
response = _revert(client, auth_headers, appid)
assert response.status_code == 200, response.get_json()
assert db.session.get(Application, appid).image == stored
from shopdb.core.api.applications import _appimage_dir
surviving = [name for name in os.listdir(_appimage_dir())
if name.startswith(f'application-{appid}.')]
assert surviving, 'the image file was deleted from disk'
def test_revert_removes_the_uploaded_executables(client, db, auth_headers):
app = _app(db)
appid = app.appid
assert _upload_package(client, auth_headers, appid).status_code == 200
from shopdb.core.api.applications import _apppackage_dir
packagedir = os.path.join(_apppackage_dir(), str(appid))
assert os.listdir(packagedir) == ['setup.exe']
response = _revert(client, auth_headers, appid)
assert response.status_code == 200, response.get_json()
assert not os.path.isdir(packagedir)
reverted = db.session.get(Application, appid)
assert reverted.installpath is None
assert reverted.installersha256 is None
def test_revert_leaves_an_operator_typed_install_path_alone(client, db,
auth_headers):
# A UNC path points at the site's own share, not our upload directory, and
# is not ours to wipe.
share = r'\\fileserver\software\DeviceLogger\setup.exe'
app = _app(db, installpath=share)
response = _revert(client, auth_headers, app.appid)
assert response.status_code == 200, response.get_json()
assert db.session.get(Application, app.appid).installpath == share
def test_revert_takes_the_versions_with_it(client, db, auth_headers):
app = _app(db)
appid = app.appid
db.session.add(AppVersion(appid=appid, version='1.0.0.0'))
db.session.commit()
response = _revert(client, auth_headers, appid)
assert response.status_code == 200, response.get_json()
assert AppVersion.query.filter_by(appid=appid).count() == 0
def test_revert_withdraws_the_generated_policy(client, db, auth_headers):
created = client.post('/api/applications',
json={'appname': 'Fleet App', 'isrequired': True,
'acknowledgepolicy': True},
headers=auth_headers)
assert created.status_code == 201, created.get_json()
app = Application.query.filter_by(appname='Fleet App').first()
assert ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first() is not None
response = _revert(client, auth_headers, app.appid)
assert response.status_code == 200, response.get_json()
assert ConfigurationPolicy.query.filter_by(
name=auto_policy_name(app)).first() is None
assert db.session.get(Application, app.appid).isrequired is False
def test_revert_keeps_the_inventory_links(client, db, auth_headers):
# A machine still has the software installed whether or not we ship an
# installer for it. Dropping these would make the fleet look clean.
from plugins.computers.models import Computer, ComputerInstalledApp
app = _app(db)
computer = Computer(assetid='PC-001')
db.session.add(computer)
db.session.commit()
db.session.add(ComputerInstalledApp(computerid=computer.computerid,
appid=app.appid))
db.session.commit()
response = _revert(client, auth_headers, app.appid)
assert response.status_code == 200, response.get_json()
assert ComputerInstalledApp.query.filter_by(appid=app.appid).count() == 1
def test_revert_is_refused_without_the_typed_confirmation(client, db,
auth_headers):
app = _app(db)
assert _upload_package(client, auth_headers, app.appid).status_code == 200
response = _revert(client, auth_headers, app.appid, confirm=None)
assert response.status_code == 400, response.get_json()
# Nothing may have been removed on a refused call.
from shopdb.core.api.applications import _apppackage_dir
assert os.path.isdir(os.path.join(_apppackage_dir(), str(app.appid)))
def test_revert_is_refused_when_the_confirmation_is_wrong_case(client, db,
auth_headers):
app = _app(db)
response = _revert(client, auth_headers, app.appid, confirm='ok')
assert response.status_code == 400, response.get_json()
def test_revert_requires_the_edit_permission(client, db, member_headers):
app = _app(db)
response = _revert(client, member_headers, app.appid)
assert response.status_code in (401, 403), response.get_json()

View File

@@ -77,11 +77,15 @@ def test_installer_upload_sets_the_install_path(client, db, auth_headers):
assert response.status_code == 200, response.get_json()
body = response.get_json()['data']
# The installer keeps the name the vendor shipped, namespaced by
# application id so a generic "setup.msi" from two vendors cannot collide.
# The old application-<id>.msi told the operator nothing and made a
# re-import look like new software.
assert body['installpath'] == \
f'/api/applications/package/application-{app.appid}.msi'
# The name the vendor shipped is reported back, since the stored copy is
# renamed and the uploader needs to recognise what they sent.
f'/api/applications/package/{app.appid}/setup.msi'
assert body['uploadedfilename'] == 'setup.msi'
# An MSI is not a PE file, so there is no version resource to read.
assert body['detectedversion'] is None
def test_installer_download_needs_a_token(client, db, auth_headers):
@@ -90,7 +94,7 @@ def test_installer_download_needs_a_token(client, db, auth_headers):
client.post(f'/api/applications/{app.appid}/package',
data={'file': (io.BytesIO(b'fake installer'), 'setup.msi')},
content_type='multipart/form-data', headers=auth_headers)
path = f'/api/applications/package/application-{app.appid}.msi'
path = f'/api/applications/package/{app.appid}/setup.msi'
assert client.get(path).status_code == 401

View File

@@ -0,0 +1,96 @@
"""Tests for reading an installer's version at upload time.
Duplicate application and version rows were being created because an operator
imported the same installer twice: nothing on screen told them the file already
matched a version in the catalogue. Reading the version out of the installer at
upload closes that, and saying so when there is no version to read closes the
rest of it.
Building a real PE with a RT_VERSION resource inline is impractical (see the
agentstore unit tests, which cover the parser itself), so these tests
monkeypatch read_pe_file_version and focus on the wiring: that the upload route
calls it, reports what it found, and reports None rather than guessing.
"""
import io
from shopdb.core.models import Application
from plugins.endpoints.api import agentstore
_PE_BYTES = b'MZ' + b'\x00' * 128
def _app(db, name='Test App'):
app = Application(appname=name)
db.session.add(app)
db.session.commit()
return app
def _upload(client, headers, appid, name='setup.exe', data=_PE_BYTES):
return client.post(f'/api/applications/{appid}/package',
data={'file': (io.BytesIO(data), name)},
content_type='multipart/form-data',
headers=headers)
def test_upload_reports_the_detected_version(client, db, auth_headers,
monkeypatch):
monkeypatch.setattr(agentstore, 'read_pe_file_version',
lambda data: '2.4.1.0')
app = _app(db)
response = _upload(client, auth_headers, app.appid)
assert response.status_code == 200, response.get_json()
assert response.get_json()['data']['detectedversion'] == '2.4.1.0'
def test_upload_reports_none_when_there_is_no_version_resource(
client, db, auth_headers, monkeypatch):
# None is the signal the form uses to ask the operator for a version. It
# must not be substituted with a placeholder, or the duplicate-import
# problem comes straight back.
monkeypatch.setattr(agentstore, 'read_pe_file_version', lambda data: None)
app = _app(db)
response = _upload(client, auth_headers, app.appid)
assert response.status_code == 200, response.get_json()
assert response.get_json()['data']['detectedversion'] is None
def test_detection_does_not_consume_the_stream(client, db, auth_headers,
monkeypatch):
# Version detection reads the upload before it is saved. If it did not
# rewind, the stored installer would be truncated or empty - which would
# not fail any assertion about the version, only corrupt the payload.
monkeypatch.setattr(agentstore, 'read_pe_file_version',
lambda data: '1.0.0.0')
app = _app(db)
assert _upload(client, auth_headers, app.appid).status_code == 200
download = client.get(
f'/api/applications/package/{app.appid}/setup.exe',
headers=auth_headers)
assert download.status_code == 200
assert download.data == _PE_BYTES
def test_detection_failure_does_not_fail_the_upload(client, db, auth_headers,
monkeypatch):
# A version is a convenience; losing it must not cost the operator the
# upload of a 500MB installer.
def _boom(data):
raise ValueError('malformed resource directory')
monkeypatch.setattr(agentstore, 'read_pe_file_version', _boom)
app = _app(db)
response = _upload(client, auth_headers, app.appid)
assert response.status_code == 200, response.get_json()
assert response.get_json()['data']['detectedversion'] is None

View File

@@ -0,0 +1,156 @@
"""Tests for the migration that nests uploaded installers by application id.
The migration rewrites installpath for every affected row and moves the file on
disk. Both halves matter: a row left on the old scheme is a download that 404s
against a route that no longer serves it, and a file left behind is a download
that 404s against a row pointing at nothing.
These call upgrade()/downgrade() directly with a stubbed bind rather than
running Alembic, so the file movement can be checked against a real temporary
directory without a database round trip.
"""
import importlib.util
import os
import pytest
def _load_migration():
path = os.path.join('migrations', 'versions', '7d46_nest_app_packages.py')
spec = importlib.util.spec_from_file_location('nest_app_packages', path)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module
class _FakeResult:
def __init__(self, rows):
self._rows = rows
def fetchall(self):
return self._rows
class _FakeBind:
"""Records UPDATEs and replays a fixed SELECT, so no DB is needed."""
def __init__(self, rows):
self._rows = rows
self.updates = []
def execute(self, statement, params=None):
text = str(statement)
if text.strip().startswith('SELECT'):
return _FakeResult(self._rows)
self.updates.append(params)
return _FakeResult([])
@pytest.fixture
def migration():
return _load_migration()
def _patch(monkeypatch, migration, bind, directory):
monkeypatch.setattr(migration.op, 'get_bind', lambda: bind)
monkeypatch.setattr(migration, '_package_dir', lambda: directory)
def test_upgrade_moves_the_file_and_repoints_the_row(monkeypatch, migration,
tmp_path):
directory = str(tmp_path)
flat = os.path.join(directory, 'application-7.msi')
with open(flat, 'wb') as handle:
handle.write(b'installer bytes')
bind = _FakeBind([(7, '/api/applications/package/application-7.msi')])
_patch(monkeypatch, migration, bind, directory)
migration.upgrade()
assert not os.path.exists(flat)
moved = os.path.join(directory, '7', 'installer.msi')
assert os.path.isfile(moved)
with open(moved, 'rb') as handle:
assert handle.read() == b'installer bytes'
assert bind.updates == [
{'path': '/api/applications/package/7/installer.msi', 'appid': 7}]
def test_upgrade_repoints_the_row_even_when_the_file_is_missing(
monkeypatch, migration, tmp_path):
# A row left on the old scheme is unserviceable and cannot be repaired by
# re-uploading, so the rewrite must not be conditional on the file.
bind = _FakeBind([(9, '/api/applications/package/application-9.exe')])
_patch(monkeypatch, migration, bind, str(tmp_path))
migration.upgrade()
assert bind.updates == [
{'path': '/api/applications/package/9/installer.exe', 'appid': 9}]
def test_upgrade_leaves_already_nested_rows_alone(monkeypatch, migration,
tmp_path):
# Re-running the migration must not rewrite a path that is already correct,
# which would clobber a real vendor filename with "installer.exe".
bind = _FakeBind([(3, '/api/applications/package/3/DeviceLoggerSetup.exe')])
_patch(monkeypatch, migration, bind, str(tmp_path))
migration.upgrade()
assert bind.updates == []
def test_upgrade_ignores_an_operator_typed_share_path(monkeypatch, migration,
tmp_path):
# The SELECT only matches our own prefix, but prove a UNC path is untouched.
bind = _FakeBind([])
_patch(monkeypatch, migration, bind, str(tmp_path))
migration.upgrade()
assert bind.updates == []
def test_downgrade_puts_the_file_back(monkeypatch, migration, tmp_path):
directory = str(tmp_path)
nested = os.path.join(directory, '7')
os.makedirs(nested)
with open(os.path.join(nested, 'installer.msi'), 'wb') as handle:
handle.write(b'installer bytes')
bind = _FakeBind([(7, '/api/applications/package/7/installer.msi')])
_patch(monkeypatch, migration, bind, directory)
migration.downgrade()
flat = os.path.join(directory, 'application-7.msi')
assert os.path.isfile(flat)
# The emptied directory is cleaned up so a re-upgrade does not find it.
assert not os.path.isdir(nested)
assert bind.updates == [
{'path': '/api/applications/package/application-7.msi', 'appid': 7}]
def test_upgrade_then_downgrade_round_trips(monkeypatch, migration, tmp_path):
directory = str(tmp_path)
with open(os.path.join(directory, 'application-4.exe'), 'wb') as handle:
handle.write(b'payload')
up = _FakeBind([(4, '/api/applications/package/application-4.exe')])
_patch(monkeypatch, migration, up, directory)
migration.upgrade()
down = _FakeBind([(4, up.updates[0]['path'])])
_patch(monkeypatch, migration, down, directory)
migration.downgrade()
restored = os.path.join(directory, 'application-4.exe')
assert os.path.isfile(restored)
with open(restored, 'rb') as handle:
assert handle.read() == b'payload'
assert down.updates[0]['path'] == \
'/api/applications/package/application-4.exe'

View File

@@ -0,0 +1,110 @@
"""The Fleet list must carry the asset alias.
The alias is what people actually call a PC ("Bay 3 Controller"); the hostname
is what DNS calls it. The PCs list has shown both for a long time, but the
Fleet list query never selected Asset.name, so the column could not be built
even though the data existed - it was missing rather than broken.
These pin the field on the payload and its inclusion in search, so the column
cannot quietly lose its source again.
"""
LIST_URL = '/api/endpoints/'
def _endpoint(db, hostname='fleetbox', alias='Bay 3 Controller'):
"""An Asset + Computer + EndpointState triple: one row on the fleet list.
The list joins through Computer and filters on Asset.isactive, so all three
are required for a row to appear at all.
"""
from shopdb.api import Asset, AssetType
from plugins.computers.models import Computer
from plugins.endpoints.models import EndpointState
assettype = AssetType.query.filter_by(assettype='computer').first()
if assettype is None:
assettype = AssetType(assettype='computer', pluginname='computers',
tablename='computers')
db.session.add(assettype)
db.session.flush()
asset = Asset(assetnumber=hostname, name=alias,
assettypeid=assettype.assettypeid, isactive=True)
db.session.add(asset)
db.session.flush()
db.session.add(Computer(assetid=asset.assetid, hostname=hostname))
db.session.add(EndpointState(assetid=asset.assetid, hostname=hostname))
db.session.commit()
return asset.assetid
def _rows(response):
return response.get_json()['data']
def test_the_fleet_list_carries_the_alias(client, db, auth_headers):
_endpoint(db)
response = client.get(LIST_URL, headers=auth_headers)
assert response.status_code == 200, response.get_json()
rows = _rows(response)
assert len(rows) == 1
# Exposed as 'alias', not 'name': the row already has a hostname, and
# 'name' beside 'hostname' reads as a duplicate of the same thing.
assert rows[0]['alias'] == 'Bay 3 Controller'
def test_an_endpoint_without_an_alias_still_lists(client, db, auth_headers):
# The column renders '-' for these. A null must not drop the row or 500.
_endpoint(db, hostname='namelessbox', alias=None)
response = client.get(LIST_URL, headers=auth_headers)
assert response.status_code == 200, response.get_json()
rows = _rows(response)
assert len(rows) == 1
assert rows[0]['alias'] is None
assert rows[0]['hostname'] == 'namelessbox'
def test_the_alias_is_searchable(client, db, auth_headers):
# Someone looking for a PC types what it is called, not its hostname.
_endpoint(db, hostname='pc-a', alias='Bay 3 Controller')
_endpoint(db, hostname='pc-b', alias='Paint Booth')
response = client.get(LIST_URL + '?search=Bay 3', headers=auth_headers)
assert response.status_code == 200, response.get_json()
rows = _rows(response)
assert [r['hostname'] for r in rows] == ['pc-a']
def test_searching_by_hostname_still_works(client, db, auth_headers):
# The alias clause was added to an existing OR; prove it did not replace
# what was already there.
_endpoint(db, hostname='pc-a', alias='Bay 3 Controller')
_endpoint(db, hostname='pc-b', alias='Paint Booth')
response = client.get(LIST_URL + '?search=pc-b', headers=auth_headers)
assert response.status_code == 200, response.get_json()
rows = _rows(response)
assert [r['hostname'] for r in rows] == ['pc-b']
def test_the_row_keeps_its_other_columns(client, db, auth_headers):
# The alias was added mid-tuple in the query, so every following column
# shifted position. If that unpacking is wrong the values silently swap
# rather than raising.
_endpoint(db, hostname='fleetbox', alias='Bay 3 Controller')
response = client.get(LIST_URL, headers=auth_headers)
row = _rows(response)[0]
assert row['hostname'] == 'fleetbox'
assert row['assetnumber'] == 'fleetbox'
assert row['alias'] == 'Bay 3 Controller'
assert row['status'] in ('never', 'quiet', 'reporting')
assert 'computerid' in row and row['computerid'] is not None