The source check of a new logical or NFS pool accepted only letters,
digits and "/", which refused paths such as /dev/mapper/vg0-data,
/dev/disk/by-id/wwn-..., /dev/disk/by-path/pci-0000:00:1f.2-ata-1 or
/export/vm_images. The target check let any first character but "-"
through, so a quote, "<" or "&" reached the pool XML and libvirt failed
on it.
Target and source now share one expression: letters, digits, "/", "_",
".", ":" and "-" (not first, not right after "/"), still in linear time.
The Vagrant computes' NFS export becomes /srv/wvc-nfs, created through the
page in the live pool test.
create_storage_netfs started the pool without a build, so a netfs pool
whose target directory was missing failed to mount ("mount point ... does
not exist"): the page showed the error and the pool stayed defined and
inactive. It now starts with VIR_STORAGE_POOL_CREATE_WITH_BUILD, as the
dir and logical pools do, which makes the mount point.
The Vagrant computes export /srv/wvcnfs to 127.0.0.1, and the live pool
test creates an NFS pool on it at a missing mount point when
TEST_LIBVIRT_NFS_EXPORT is set.
create_storage built only logical pools, so a dir pool whose target path
was missing failed to start ("cannot open directory"): the page showed the
libvirt error and the pool stayed defined, inactive and without autostart.
The pool is now started with VIR_STORAGE_POOL_CREATE_WITH_BUILD, which
makes the directory, or for a logical pool the volume group as the
separate build call did.
A live test creates a dir pool at a missing path on the storages page;
the LVM live test still creates its logical pool the same way.
Rendered through every URL on libvirt's test driver, the pages had
duplicate IDs and markup browsers repaired silently:
- Each flavor modal's "Advanced" button toggled #flav-add-collapse, the
first modal's section; every modal now has its own. The flavor and
template copies of the custom tab's control IDs, and the template
tab's unused images input, are gone.
- The VM page's action icons and the XML editors were styled by
repeated IDs; they use classes.
- Modal forms opened inside modal-body and closed after modal-footer;
they wrap both. Stray end tags and duplicate attributes are removed,
paragraphs no longer hold modals or rows, table heads have a row, and
the create page closes its row and column.
- html lang is the active language instead of "en".
A test walks every URL as a superuser and fails on a repeated ID. The
console page still has no doctype: its full-height layout relies on
quirks mode and needs its own check.
The job panel follows an ISO upload, so the dialog closes when the first
chunk is accepted and its progress bar goes; the dialog opens again with
the error if the upload stops. The browser still sends the file, so the
page asks before it is left while an upload runs. The storage page no
longer includes the please-wait dialog, which nothing on it opened.
The upload session lives in a job instead of flocks and .meta/.done
files; the job panel shows and cancels it; an upload idle for an hour is
ended and its partial file deleted; the page no longer reloads after an
upload.
libvirt deletes any volume it is asked to. The storage page and the API
deleted a VM's disk, the base image of another volume or the file a
snapshot was taken on without a check, breaking the VM or its
snapshots.
wvmStorage.del_volume now refuses a volume a domain on the compute uses
as a disk or a backing file of one, a file of a domain's snapshots, or
the backing file of another volume. The check is the one destroy
already used, moved to wvmConnect and extended to snapshot files, so
destroy also keeps the files of another VM's snapshots. The storage
page shows the refusal; the API answers 400 with it, as for other
refusals. Deleting a disk from a VM refuses after the detach when the
VM's own snapshots need the volume, and says the volume was kept.
Network disks count by their source name, pool volume disks by their
volume's path. A volume whose metadata cannot be read stops the check
instead of counting as unused; only one removed meanwhile is skipped.
The check and the delete hold the compute lock, so no VM operation
attaches the volume in between; a busy compute is a message on the page
and 409 in the API.
131 test modules sat next to the code they test; instances alone had
51 of them beside views.py and utils.py. They move into a
tests/ package per app, and the JavaScript checks into
webvirtcloud/tests/js. Each non-empty tests.py becomes tests/test_views.py;
the empty generated stubs are removed.
Imports of the app's own modules become absolute, paths built from
__file__ go one level up, the live helper is instances.tests.livetest,
and coverage omits */tests/*. The suite discovers the same 960 tests as
before.
Each 32 MiB chunk is its own request and opened a new SSH connection and
SFTP session (~200 ms). A worker now keeps its connection to the host for
the next chunk; each request still opens its own SFTP session (~50 ms),
so an SFTP error never reaches the next one. A connection is not reused
after an error, when the host dropped it or once it is 10 minutes old; an
idle one is closed after a minute, and a forked worker leaves the parent's
to the parent. The SFTP start, which paramiko waits for without a deadline,
is ended after 10 s. Live: 256 MiB in 8 chunks 4.5-4.9 s -> 3.7 s.
Each 32 MiB chunk went out as 1024 SFTP writes, each waiting for its
answer: 19 MiB/s on a gigabit LAN, ~1.9 s per chunk with the SSH session.
The writes now go out without waiting and their answers are read as they
come and at the end: ~0.53 s per chunk, the same file (checked by sha256).
paramiko's own pipelined writes (set_pipelined) drop the errors in their
answers, so a full disk would leave a short ISO behind a successful
upload. Each write is registered to its file instead, the way paramiko
does for prefetched reads: every answer, in any order, reaches the file
and an error is raised as soon as one is read.
Reviewing the error handlers outside instances/ turned up three that masked or
swallowed the failure they caught:
- appsettings/settings.get_settings() runs on every request (middleware) and
loads app_settings from the database. Its bare `except: pass` left `entries`
unbound on a database error, so the next line raised UnboundLocalError
instead; it also caught KeyboardInterrupt/SystemExit. It now catches
DatabaseError and returns, keeping the already-loaded values.
- vrtManager.network.refresh_dhcp_leases raised a formatted string
(`raise f"..."`), which Python rejects with TypeError, hiding the real
DHCPLeases failure. It now raises util.OperationError, which the API/HTML
error handling already renders as a libvirt error, and clears self.leases.
- storages.views.get_volumes caught libvirtError and answered 200 with an empty
body, so the caller could not tell a failed pool read from an empty pool. It
now returns 400 with the error.
The other broad handlers reviewed (appsettings/nwfilters/virtsecrets views,
storages/upload cleanup and recovery paths, vrtManager display getters) report
the error or are best-effort and were left as they are.
Tests: get_settings() survives a database error during the query and during
iteration without raising and still loads a real value; refresh_dhcp_leases
raises OperationError (keeping the cause) not TypeError; get_volumes returns 400
on a listing failure and the sorted volumes on success. The tests fail on the
previous code.
Nothing limited failed logins: a password could be guessed at the speed
the server answers. django-axes (8.3.1, Django 5.2, Python 3.10-3.13)
now counts them.
- A user is locked for the address the failed logins come from
(username and IP together), so an attacker cannot lock someone out
from elsewhere and users behind one NAT address do not share a lock.
A successful login resets the count. axes sits in front of every
authentication: the login page with or without OTP, LDAP and the API.
- The number of failed logins (default 5) and the lock time in minutes
(default 15) are on the Settings page (LOGIN_FAILURE_LIMIT,
LOGIN_LOCKOUT_MINUTES, added by migration appsettings.0014) and apply
at once; an invalid value falls back to the default. Unlock with
"manage.py axes_reset_username <username>".
- Behind the bundled nginx every request comes from 127.0.0.1; the
client address is read from X-Real-IP, which nginx sets, but only for
requests from a loopback address or from LOGIN_TRUSTED_PROXIES, so a
forged header does not escape a lock.
settings.py.template gets the app, middleware, backend and AXES_*
settings; README tells existing installations what to copy. Tests that
call client.login now pass a request, which the axes backend needs.
The new tests fail with axes disabled.
The expression that checks a storage pool's target directory nested a
repetition inside another, so a crafted target backtracked
exponentially: "," followed by 22 dashes and "!" took 1.3 s, and every
two more characters four times as long. The field allows 100
characters, so one form post could hold a worker for good (CodeQL
py/redos; the page is for superusers).
The new expression has no nested repetition and accepts exactly the
same targets: compared with the old one on 200,000 random strings over
letters, digits, "/", ".", "-", "_", ",", "!", space and newline, they
agree on every string. A crafted 100-character target is now checked
in microseconds. The tests pin the accepted and refused targets and
time the crafted one.
Web views:
- 34 views returned to request.META["HTTP_REFERER"] + "#tab": without
a Referer they failed with a TypeError (500), and a Referer from
another site was followed (open redirect). They return through
_back(), which keeps a same-site Referer and otherwise opens the VM
page at the tab. get_safe_redirect uses the same check. Rejected disk
options also return to the VM's disks tab instead of the VM list.
- migrate_instance returned silently when the target was the current
compute or unreachable, so the web view logged "Instance is migrated"
and the API answered "instance migrate is started". It now raises;
the view reports the error and logs only a completed migration, and
an unknown compute id is a 404 instead of a 500. The automatic
migration after a clone skips a target that is the current compute.
- add_owner and del_owner failed with a 500 without their ids, add_owner
also when the user was already an owner, and del_owner deleted the
given ownership record even when it belonged to another VM. Ids are
validated, a record of another VM is a 404, and a repeated owner is
reported.
- Broad handlers that hid programming errors are narrowed: clone catches
libvirt, value and OS errors (a lock timeout included), ownership
lookups catch DoesNotExist. The DRBD status no longer needs its bare
except: the VM name is escaped in the regex, which was the only thing
that could fail there.
API:
- A libvirt error under /api/ is JSON with status 400, and a lock
timeout 409, instead of the HTML error page. This lives in the
existing ExceptionMiddleware, so existing settings need no change.
- Volume create/delete on an inactive pool answer 409, and invalid
volume data 400 with the serializer errors, instead of 200.
The new tests cover every view that returns with _back() without a
Referer and with a foreign one, the migrate failures on web and API,
the owner ids, the API error format and the volume statuses; nine of
the ten fail on the previous code.
Super-linter ran from the unpinned :latest image and, because of
branches-ignore: [master], never on pull requests to master. Ruff now
checks syntax errors and pyflakes rules (E9, F) on every push and PR.
Fixes the existing findings: unused imports and variables, an unused
duplicate import in instances/tests.py, and the star import in
accounts/views.py. settings-dev.py keeps its star import of the base
settings.
The upload session was recorded only after the first chunk had been
written, and a retried first chunk was refused when its part file was
missing. The session (with its upload id) is now written before the
part file is created, the part file is created by whichever chunk finds
none, and a missing part file is an error only after the first chunk.
An interrupted first chunk can be sent again without adopting an
unrelated file.
- Routers use path converters (use_regex_path=False): computes, instances
and flavors take <int:...> ids, so non-numeric ids get 404 and
drf-spectacular derives the parameter types itself.
- Each method declares what it really returns: detail serializers,
a shared StatusSerializer for status-only replies, no request body for
actions. Instance properties used by the details serializer get return
type hints.
- The create-instance API drops the firmware field. Its only accepted
value was "" (BIOS), which produced an empty, invalid enum, and the
UEFI branch behind it was unreachable. Clients that still send it are
unaffected; DRF ignores unknown fields.
The schema now passes spectacular --validate --fail-on-warn; a test
keeps it that way.
Several views returned json.dumps() output through HttpResponse, i.e. as
text/html, and guess_mac_address reflects the URL's vname into it
(reflected XSS). Templates filled the log table and volume dropdowns by
concatenating server data into HTML strings, and log messages contain
user-entered snapshot/disk/clone names (stored XSS that runs when a
superuser opens the Logs tab).
- Use JsonResponse in guess_mac_address, random_mac_address,
guess_clone_name, sshkeys, storages.get_volumes,
datasource.os_metadata_json and logs.vm_logs; sshkeys?plain=true is
served as text/plain.
- stats_tab.html builds log rows with .text(); instance.html and
create_instance_w2.html build <option> elements with .val().text().
- Computes API: mark password as write_only in ComputeSerializer, restrict ComputeViewSet and architectures to IsAdminUser.
- Instances API: enforce user ownership tenant isolation in InstanceViewSet.list, restrict MigrateViewSet and CreateInstanceViewSet to IsAdminUser, require IsAdminUser for mutating FlavorViewSet.
- Storage, Network, and Interface APIs: restrict StorageViewSet, VolumeViewSet, NetworkViewSet, InterfaceViewSet to IsAdminUser.
- AppSettings & NWFilters: protect appsettings and nwfilter views with @superuser_only (403 Forbidden for non-superusers).
- noVNC Console: sign tokens with TimestampSigner (salt='console.novnc', max_age=600), verify signature and user authorization in novncd with backward-compatible fallback for debug/test mode.
- Accounts: avoid hardcoded default admin password; generate cryptographically secure random password unless ADMIN_PASSWORD is set (preserve 'admin' in test runner).
- gstfsd daemon: bind to 127.0.0.1 by default instead of 0.0.0.0.
- Tests: added comprehensive test coverage for API security, token expiration/rejection, and view permissions (132 tests passing).
* Add rest framework for API: First Commit
* modify some shell scripts to make variable references safer; modify some python scripts to reduce the code complexity and cyclomatic complexity of functions.
* Add REST API for some webvirtcloud functions. Instance list/delete/create, compute list/delete/create, storages-network list/retrieve. Add swagger and redoc for API interface
* update requirements
Co-authored-by: herengui <herengui@uniontech.com>
