* oidc: apply custom roles from group mappings instead of dropping them
A group mapping onto a custom (e.g. tenant-scoped) role never took effect. The
mapping loop ranked roles through a hard-coded {viewer: 0, user: 1, admin: 2}
table read with .get(role, 0), so any custom role tied with viewer and the
strict `>` comparison never fired — while the log line still reported
"Custom group mapping matched", leaving no trace of why the user ended up on
oidc_default_role. Custom roles are offered in the config UI (the Default role
dropdown lists every non-builtin role) and accepted by the settings validator,
so the intent was clearly that they work.
Rank custom roles between user and admin: an explicit mapping onto a custom
role now beats the coarse viewer/user defaults, but can never demote someone
matched by the admin group. A role that came from oidc_default_role is also
tracked separately, so a custom default no longer swallows every mapping.
Adds tests/test_oidc_group_mapping.py — the four custom-role cases fail before
this change, the four no-regression cases (admin precedence, builtin ladder,
unmatched default) pass both before and after.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(oidc): don't let a matched mapping demote a configured admin default
Review feedback on #682: role_from_default let any matching mapping replace the
configured default, including when that default is admin — so an install using
oidc_default_role='admin' would be downgraded to viewer/user/a custom role on the
first matching group. The effect is a loss of admin access rather than an
escalation, but it changes long-standing behaviour and can leave an install
without access to admin-only workflows.
Keep the rule that the configured default stands for "no group matched" and may
therefore be replaced by a group that did match, but exempt admin from it. Adds
the regression test, which fails without the exemption.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(oidc): warn when several custom-role group mappings match
Review feedback on #682: custom roles all share one precedence level, so when a
user matches more than one custom-role mapping the first entry in the list wins
and the rest tie out — the outcome depends on mapping order.
Keeping first-match-wins: the mapping list is an ordered list in the UI, so
reading it top-down is the least surprising rule, and it is deterministic for a
given config. Defining a real precedence *between* custom roles would mean
ranking their permission sets against each other, which is a product decision
(permission count is a poor proxy for privilege, and tenant-scoped roles are not
comparable at all) rather than something to settle inside a bug fix.
What was worth fixing is the silence: several matches now log a warning naming
the roles, the one applied, and how to change the outcome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The in-app updater and update.sh are a git-tree + pip file update that only fits
the source/deploy.sh layout. On an apt/dpkg install the correct path is apt
upgrade (deps are system python3-* packages; a pip run diverges from dpkg and
can't lift the dpkg-owned crypto libs -> fail-closed TLS on restart); on Docker
a file update is discarded at the next image pull.
- settings.py: _detect_install_method() (docker via /.dockerenv + cgroup, apt via
dpkg -S, else source); perform_pegaprox_update refuses on apt/docker with the
right guidance + copy-paste command (409, allow_managed override); check-update
reports install_method / in_app_update_supported / managed_update_{hint,command}.
- update.sh: same detection + guard before touching anything (--force override).
- settings_modal.js: the Install button becomes a package-manager / image hint
with a copyable command on apt/docker; performUpdate handles the 409.
- tests: 5 guard/reporting tests.
- webhooks._guard_url now returns (ok, reason, url_to_use); public http(s) targets are
IP-pinned via resolve_and_pin_url so a DNS rebind between the guard check and the POST
can't swing the request to an internal host. allow_private opt-in skips the pin so an
internal ntfy/Gotify keeps working. Both send_to_channel / _post_ntfy callers use the
pinned url. (Aikido #469089218)
- templates_lib deploy: re-run the SSRF guard on tpl['image_url'] right before the node
wgets it. add_custom_template validates on entry, but built-in catalog and DB-stored
URLs reached the sink unchecked. (Aikido #469089270)
- tests: webhook-guard 3-tuple contract + metadata/loopback block + allow_private no-pin.
- _authz_backup_targets: a non-admin backup.schedule holder now needs an
explicit include-list of VMs they own. all=1 / pool / exclude-mode /
selMode=all|exclude|pool and an *empty* selection are admin-only — PVE
treats several of those as "every VM", and the load->edit->save round-trip
of an admin job re-hits the gate on PUT so a scoped user can't retune it.
- get_vms_without_pool: a single non-numeric vmid no longer int()-throws and
500s the whole unpooled-VM listing; skip the malformed row instead.
- 2 regression tests (empty/exclude selection + PUT all=1 denied).
- push.py _is_internal_or_metadata_host (#469089273): stopped splitting the host on
':' (which mangled every IPv6 literal, '::1' -> '', bypassing the block) and unwrap
IPv4-mapped IPv6 (::ffff:a.b.c.d) so metadata/private checks apply. +6 unit tests.
- docker.yml + release-images.yml (#348463387/#348463388): persist-credentials:false on
the actions/checkout steps that never push.
The read routes already call check_vmware_access / check_pbs_access, but the
mutating ones were guarded only by @require_auth(perms=[...]):
- vmware.py update/delete/diagnose (#469089245/#207/#204): a vmware.config holder
could take over, delete, or probe another tenant's ESXi server by id.
- pbs.py update/delete (#469089284/#244) and auto-storage (#469089213): a
pbs.config holder could overwrite/delete another tenant's PBS record, or push
its stored credentials onto an arbitrary cluster's storage config.
Each now calls check_* first (admins + unlinked servers unaffected); auto-storage
also check_cluster_access on every target cluster. +6 regression tests.
Low-severity items from the Testing-branch audit:
- power/cost list_rates: scope by the token's floored effective_role, not the
owner's account role, so an admin-owned but scoped token can't enumerate every
cluster's rates (mirrors the upsert admin check).
- i18n relative-time: 'timeAgo' was always appended ('5s vor' in German). Split
into locale-ordered timeAgoSec/timeAgoMin templates for all 8 locales
(de/fr/es/pt lead, en/zh/ko/it trail).
- release-images CI: arm64 builds under qemu emulation; a failed arm64 leg no
longer blocks the amd64 release (continue-on-error on the arm64 matrix legs;
publish already attaches whatever artifacts exist).
- tests: BMC in-band read key->agent->password fallback order (#609) now has
direct coverage (3 tests).
(OIDC cross-source collision finding intentionally left alone — OIDC is hands-off.)
Follow-ups from the full Testing-branch audit:
- add_disk (LXC): a caller-supplied but already-occupied mpN — or 'rootfs' —
was written straight through, REPLACING an existing mountpoint and orphaning
its volume. Now coerce an occupied / rootfs / QEMU-shaped id to the next FREE
mpN, and reject a mount path containing ',' or '=' (mountpoint-option
injection). (+3 regression tests)
- hardening UI: the verbose-mode select-unapplied handler treated the per-control
{status,evidence} object as truthy, so it never pre-selected in verbose mode.
Made it object-aware, matching checkHardening.
- storage: the ISO/template 'From URL' buttons gated on storage.download, but the
download-url endpoint enforces storage.upload — aligned the UI gate.
- i18n(zh): dropped 10 backfill keys that duplicated ones already later in the zh
map (last-wins made them dead; topTalkers text never took effect).
Same gap as the power-rate fix: GET /api/cost/rates listed every cluster's
cost rates to any authed user, and GET /api/cost/rates/<id> had no tenant gate.
Scope the list via get_user_clusters (keep __default__) and add the
check_cluster_access gate to the single-cluster read, mirroring api/power.py.
+ 3 regression tests.
- client_portal _vm_power: reboot was gated on vm.start, so a start-only
portal user could reboot (stop+start) a guest. Now gates on vm.restart,
matching the main VM API and the scheduler. (Aikido #469089251)
- api/power.py list_rates (GET /api/power/rates): returned every cluster's
power/cost rates to any authed user. Now scoped via get_user_clusters
(admins / default tenant see all), always keeping the shared __default__
fallback row. (Aikido #469089241)
- tests: tests/test_aikido_batch3.py (5) — scoped vs admin list + the
reboot/start/shutdown permission mapping.
- add_disk: containers were sent scsi0/scsiN keys, which PVE rejects
("property is not defined in schema"). Coerce any non-mp id onto the
next free mpN and always pass a container-side mount path (mp=).
- AddDiskModal: for CTs default to the next mp slot, drop the QEMU
bus/format/iothread/ssd controls, add a Mount-Path field (+ DE/EN i18n).
- unlock_vm: plain delete=lock failed for clusters wired up with a root
API token ("Only root may use this option"). Send skiplock when
root@pam and fall back to qm/pct unlock over SSH when the API cannot.
- tests: mpN coercion + slot picking + default mount path + QEMU
untouched; skiplock / token->SSH fallback / not-locked / both-fail.
Brings rmalchow s #651 (keep the full preferred_username so bob@corp.com and
bob@partner.com no longer collapse onto one account) onto Testing. It had been
merged to main by mistake (a silent gh retarget failure), so merge main back in
to land it where our work actually ships and keep Testing a descendant of main.
The in-band BMC read now tries key -> agent -> password SSH (26b313d). The fake
manager is a MagicMock, so mgr.config.ssh_key was a truthy auto-attribute and the
read went down the (unstubbed) key branch instead of the agent path the hardware
tests stub + assert -> test_read_after_consent_returns_hardware_200 got available=False.
Real PegaProxConfig.ssh_key is a string (empty when unset); set it to '' on the fake
so it models a node with no key configured. Full suite 401 green.
Second half of the adversarially-verified findings (batch 1 = 35d4078). Each fixed,
code-reviewed and covered by tests/test_aikido_batch2.py (17 new, full suite 401 green).
- power: only a global admin (effective_role) may overwrite the shared __default__
power-rate row — a cluster.config holder edits only its own cluster
- metrics exporter: /api/metrics requires an admin-role token, not any valid token
(it emits cluster-wide, cross-tenant infra gauges)
- portal: build_authz_user in _vm_power so a token's effective_role is honoured;
invalidate the user's other sessions on portal password change
- cluster-groups: treat a global (tenant_id NULL) group as admin-only for the
delete + balance-now writes, matching the earlier update fix
- vm-tags: reject a non-numeric vmid before the global DELETE+rewrite, and roll back
save_vm_tags on error so a mid-loop failure can't persist a partial table wipe
- datacenter/multipath: allowlist the path_selector policy, and reject non-member
nodes before SSH (no more `_get_node_ip(node) or node` fallback to a raw hostname)
- storage: pin http download-url fetches to the validated IP (DNS-rebind); https is
left as the hostname since the node's TLS cert check already defeats a rebind
- multi-sdn: advertise an in-flight span's zone/controller so a concurrent purge
can't tear down infra a create is still building (TOCTOU)
- ws-token validate: enforce node.shell for the standalone node SSH shell path
(shell=node) — the VM termproxy path is unaffected
- SSE: scope vmware_vms / vmware_vm_detail to the server's linked_clusters instead
of broadcasting guest_info/performance to every client; scope the portal audit
task feed to the cluster it happened on (portal writers now set cluster=)
- LDAP: authoritative re-sync — rebuild LDAP-sourced perms/tenant_permissions from
the current group mapping instead of only unioning them in, so group removal revokes
Adversarially verified all 45 AI-pentest findings against the current code; this batch
fixes the 10 HIGH + 2 MED confirmed REAL (17 were by-design/known admin-wide debt, 2
already fixed, 1 false positive). Full in-process test suite is green — the one case that
failed was a test that encoded the very BMC credential-exfil this now guards; corrected it
and added the negative case.
- rbac.py: user_can_access_vmware_vm honours effective_role (token-scoped), like its Proxmox
twin, so an admin-owned viewer-scoped API token no longer gets the full-admin VMware bypass.
- nodes.py / vmware.py: reject a masked password paired with a CHANGED host - the stored BMC /
VMware secret can no longer be shipped to a caller-chosen host (credential exfiltration).
- schedules.py: create/update enforce the same per-VM ACL as the live action (build_authz_user
+ user_can_access_vm), not just cluster reachability.
- pbs.py: restore (overwrite/test) enforces per-VM authorization on the destination VMID;
backup-diff now requires pbs.datastore.view instead of plain pbs.view.
- users.py: create/update cap delegated permissions and custom roles to what the caller holds
(a tenant admin.users delegate can no longer mint accounts with admin.* perms via the
permissions[] list or the custom-role tier fallback); update_user rejects a foreign-tenant
custom role; update_tenant is tenant-scoped like get_tenant_quota.
- groups.py: a global (tenant_id NULL) cluster group is admin-only for writes.
The back-compat lookup added in the previous commit adopted an existing
account whenever its key matched the local part of the derived username. That
re-opened the very collision the commit set out to close, just one step later:
bob@partner.com logging in found the 'bob' row that bob@corp.com had
provisioned pre-change and inherited its role and tenant.
A legacy row is now only reused when it is provably the same identity -- its
auth_source is an OIDC-family one and its stored oidc_sub equals the incoming
sub claim. Local and LDAP rows are never adopted. A sub is only unique within
an issuer and no issuer is recorded on the user row, so installs that repoint
at a different IdP still have to rekey by hand; likewise a pre-change account
that never completed an OIDC login has no oidc_sub and now needs a manual
rekey rather than being silently adopted.
'+' joins the allowed char set. sanitize_username() permits it, and dropping
it merged identities the same way dropping '@' did -- a+b@example.com derived
onto a real ab@example.com account.
oidc_derive_username() takes the already-loaded users table. Both callers in
the login path had one in hand and were loading it a second time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Username derivation split preferred_username at '@', so bob@corp.com and
bob@partner.com both provisioned onto a single 'bob' account and whoever
logged in second inherited the first one's role and tenant.
Keep the whole claim. '@' also has to join the allowed char set, or dropping
the split just yields 'bobexample.com'. Nothing downstream constrains it:
sanitize_username() already permits '@', LDAP provisioning already produces
such names, username is an unconstrained TEXT PRIMARY KEY, and no username
reaches a filesystem path, shell command or interpolated SQL.
Existing accounts keep their current key via a back-compat lookup, so no
install loses permissions and nobody is locked out under auto_create_users:
false. Installs that already hold a truncated 'bob' therefore keep the
collision -- closing that needs a rekey migration, left out deliberately.
The derivation was duplicated in the auto_create_users pre-check, with a
comment recording that a previous divergence caused 403s. Both call sites now
share one helper.
Adds the first test coverage for OIDC username derivation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_ssl_bootstrap was skipped in its entirety under a root euid, but only the
cases that chmod a cert/dir to 0 actually need a non-root user (root ignores the
permission bits and they would false-fail). Moved the skipif onto just those; the
content/ENOENT/pyOpenSSL/config-guard/migration fail-closed cases now execute in a
root CI container or under `sudo pytest` too, so the #633 contract keeps real
coverage there instead of silently skipping. 19 pass locally (non-root, all run).
Also corrected the author tags on the #632/#633 comments to MK (app.py, constants.py,
core/manager.py, the test header) - they were carrying the wrong initials.
A close review of the #612 feature (backend + frontend + an in-process API run)
came back sound — auth matrix, per-member BOLA, input validation and SSRF surface
are all correct — but turned up a handful of low-severity gaps. Fixed:
Backend (api/multi_sdn.py):
- edit / reconcile / scan did a read-modify-write of per_cluster_status WITHOUT
the per-vid lock and REPLACED it from a pre-fan-out snapshot, so a concurrent
add/remove-member could lose a member entry (and edit could revert a concurrent
membership change in desired_state). New _merge_status_write() takes the per-vid
lock, re-reads inside it, MERGES the fresh results into the stored map, drops
departed members, and preserves the current member_clusters. Self-healing before,
consistent now.
- scan was gated on node.view but it overwrites the shared drift snapshot everyone
sees; gated it like every other span writer (sdn.manage + admin.settings) + the
member-access check.
- create takes a name-scoped lock + re-checks for an existing same-name record
before INSERT, so two admins racing the same name can no longer end up with two
aggregate records for one span (name IS the SDN vnet id; the fan-out is idempotent).
Frontend (vm_modals.js + dashboard.js):
- the whole EVPN feature + every mutation control was shown to node.view-only
users who then 403 on everything; added a canManage prop (isAdmin || sdn.manage
&& admin.settings) that hides Create / Edit / Re-apply / Reconcile / Scan /
Forget / Purge / add- and remove-member, leaving the read-only list. mcevpnReadOnly
note in all 7 locales.
- the validate preview rendered a member whose pre-flight SDN read FAILED as green
"ok" while the banner said "issues found"; added a p.error red branch.
- the auto-reconcile toggle reused the preposition t("on")/t("off") (no "off" key
existed) → mixed-language label; switched to the proper t("enabled")/t("disabled").
Adds tests/test_multi_sdn.py — the first committed coverage for the feature: the
API auth/validation surface (list/validate/create-4xx/404/anon-401/viewer-403 on
every write incl. scan) + the _merge_status_write merge/lock behaviour. 18 pass.
Verified: py_compile, frontend build, adversarial re-review. Live 2-cluster EVPN
E2E still owed (no real EVPN clusters). Refs #612.
When TLS was the intended posture and the cert could not be read or generated,
the bootstrap printed a WARNING and then bound plaintext HTTP on the port meant
for TLS - a silent downgrade of the management plane (TLS clients got
"Invalid http version: \x16\x03\x01..." while the service reported itself
healthy). The os.path.exists() gate also collapsed EACCES into ENOENT, so a
present-but-unreadable cert was reported "missing" and generation tried to write
over it. The config/ssl mkdir/chmod/migrate ran at import time, so a root-run
importer could plant root-owned certs and lock the service user out - the trigger
behind the reported case.
Adapted from #637 by @SpyrosPsarras, with follow-up hardening from an adversarial
review:
- Fail closed: the inline TLS setup in main() is now _resolve_ssl_context(), which
returns None only for a reverse proxy or the explicit PEGAPROX_ALLOW_PLAINTEXT=1
opt-in, and otherwise raises SystemExit. Both plaintext bind sites (gevent + the
Flask dev-server fallback) read that single context, so there is no path left
where TLS is intended yet cleartext binds.
- Unreadable != missing: readability is probed by opening the file; only a true
ENOENT leads to generation, and a present-but-unreachable cert is never
overwritten. The error names the path, dir owner/mode and the process uid/gid.
- Import-time side effects in constants.py now run only when config/ is owned by
the current euid; update.sh repairs ownership on upgrade.
- (MK) readable != loadable: _unloadable() actually load_cert_chain()s the pair in
the resolver, so a corrupt/mismatched cert gets the same actionable fail-closed
message instead of a raw ssl.SSLError crash downstream. Uses the byte-identical
call the real bind uses, so it can never reject a cert the server would accept.
- (MK) a half-present pair (one file there, the other ENOENT) fails closed instead
of regenerating over the surviving half.
- (MK) systemd StartLimitBurst so a permanently-broken cert lands in `failed`, not
an endless 5s crash-loop.
Tests: tests/test_ssl_bootstrap.py drives the real _resolve_ssl_context; the
fixture now uses a real throwaway pair so the loadability path is exercised, plus
corrupt-cert and half-pair cases. 18/19 pass here; the one gap is the
generation-SUCCESS case, blocked by this box being a broken-pyOpenSSL env (it too
fails closed) - it and a real systemd/TLS-bind E2E are owed on a proper host.
Closes#633.
The credentialed, out-of-band counterpart to in-band ipmitool — reads BMC health
over the management network via the DMTF Redfish API, as a fallback when in-band
is unavailable. A SEPARATE, sharper opt-in with an enforced 5-second delay.
* core/redfish.py — read-only Redfish reader. Pure parsers (thermal/power/system/
SEL) normalize to the SAME shape as read_node_bmc_inband, so the panel + cache +
rollup + alerting work unchanged. SSRF-guarded, GET-only, redirects refused.
* core/db.py — node_bmc_endpoints table; BMC password stored encrypted (aes256:),
masked (********) on every API response, never wiped by a masked re-save.
* core/bmc.py — REDFISH_CONSENT (v1, require_delay_seconds=5) in all 7 languages.
* api/nodes.py — redfish-consent GET/POST + per-node BMC endpoint GET/POST/DELETE/
test (admin.settings, consent-gated, SSRF-validated host). The per-node read +
cluster rollup now gate on (in-band OR redfish); the 5-min collector falls back
to Redfish for nodes with a configured endpoint (own ~1h backoff).
* Frontend — Redfish sub-panel in the node Hardware tab: enable via a 5-second-
delayed warning modal, then a BMC endpoint form (host/user/password/verify_ssl +
Save/Test/Remove). i18n across all 7 languages.
Security review (28 agents, 17 positive confirmations) — fixed the 4 confirmed:
* CRITICAL SSRF — a hostile BMC's @odata.id could userinfo-splice the credentialed
GET onto an internal host (https://<base>@169.254.169.254/...), exfiltrating the
stored BMC creds. Every follow-on hop is now urljoin'd against the validated base
and its scheme/host/port asserted equal; @/scheme/protocol-relative refs rejected.
* HIGH — config-restore could flip the new redfish consent without the audited ack;
the restore-strip now protects BOTH consent keys.
* MEDIUM — malicious BMC could OOM the reader: bodies now streamed + capped at 4 MiB.
* MEDIUM — the cluster rollup route 403'd a Redfish-only deployment; now gated on
(in-band OR redfish).
Residual (noted, not fixed): DNS-rebind TOCTOU on the base host (IP-pinning not done
autonomously per the codebase's standing SNI-pinning decision).
Tests: +33 (Redfish parsers, SSRF origin-pinning + body-cap regressions, consent +
BMC endpoint CRUD/SSRF/masking, rollup dual-gate). Full suite 347 passed.
Cluster-wide in-band BMC health, surfaced + alertable — mirrors the proven #601
temperature pipeline (5-min collector populates a per-manager cache; the 60s alert
loop only READS it, never SSHes).
Backend:
* manager.py: _node_hw_cache/_lock/_backoff + get_cached_node_hardware() and
get_cluster_hw_rollup() -> {health, available, checked, counts, degraded[]}.
* metrics.py: _node_hw_summary() SSH probe (~1h backoff for nodes without
ipmitool/BMC), populated in the 5-min collector via run_per_node(cap 8, 90s),
GATED on the compliance consent + proxmox-only. Off the hot-path.
* alerts.py: 'hardware_health' alert metric (cluster worst + per-node), ok/warning/
critical mapped to 0/1/2, auto-severity, cache-only.
* nodes.py: GET /clusters/<cid>/hardware/health rollup (consent-gated, cache-only).
* vms.py: compact hardware rollup injected into datacenter/status (cache-only,
consent-gated) so the overview badge is free.
Frontend: 'hardware_health' alert metric with a Warning-or-worse / Critical-only
threshold selector; degraded-hardware badge in the corporate sidebar, the warning
banner, and the cloud overview. i18n across all 7 languages.
Adversarial review (12 agents): 3 rejected (bounded/cosmetic), 5 confirmed & fixed:
* MED — hwHealth prop was missing at the 2nd (ungrouped) ClusterSidebarItem call
site, so the sidebar badge never showed for ungrouped clusters — now passed.
* LOW — the rollup endpoint 500'd on non-proxmox clusters — proxmox/callable guard
now degrades to an empty 'unknown' rollup.
* LOW — a '<' operator on hardware_health builds a silent no-alert rule — the UI now
only offers '>' and the create/update API pins hardware_health to '>'.
* NIT — alerts now show OK/WARNING/CRITICAL instead of the raw 0/1/2 code.
* NIT — corrected a stale '>=' code comment.
Tests: +11 (rollup endpoint gates, non-proxmox graceful, operator coercion, manager
rollup/cache unit tests). Full suite 322 passed; frontend build clean.
The compliance warning text is now localized (en/de/fr/es/pt/ko/it) while the
VERSION stays language-spanning: _HW_CONSENT_TEXT holds the same v1 warning in
each language, and hw_consent_warning(lang) injects the shared version +
require_delay_seconds so they can never drift per-language (bump all together).
* GET /api/hardware-monitoring/consent?lang=<code> returns the warning in the
requested language (falls back to English). HW_CONSENT_WARNING kept as an
English alias for existing callers/tests.
* The acknowledged LANGUAGE is now recorded alongside the version — set consent
stores ack_lang and the audit line reads 'acknowledged compliance warning v1
[de]', so the non-repudiation record captures which exact text the user saw.
* Frontend passes the current UI language to the consent GET (?lang=) and the
enable POST (lang=), and re-fetches the localized warning on language switch.
Translations adversarially verified by 6 native-level reviewers vs the English
source (control IDs CMMC/NIST 800-171 3.4.6+3.1.5/DISA STIG preserved verbatim in
every language). Applied the confirmed fixes: es 'encendido'->'alimentación' (the
operation-list 'power' means power-control, not power-on), es informar+de grammar,
de closing-quote typography, ko install-vs-may modal + 'override' precision.
Tests: +4 integration cases (per-lang warning, unknown-lang fallback, ack_lang
recorded in audit + status, unknown-lang recorded as en). Full suite 311 passed.
Step 3 — one-click ipmitool install:
* POST /api/clusters/<cid>/hardware/ipmitool/install (admin.settings) mirrors the
proven StarWind installer but with a FULLY STATIC script (no user-controlled shell
input), gated: cluster-access -> proxmox-only -> consent-required (the install
mutates the node, so it must not run before the warning is acknowledged) -> per-node
name validation -> run_per_node fan-out (cap 8, idempotent) -> audited.
Step 4 — frontend:
* Self-contained HardwareMonitoringPanel: consent fetch, mandatory compliance-warning
modal (renders the server-versioned warning text, required ack checkbox, generic
require_delay_seconds countdown ready for the Redfish phase), ipmitool install button
when missing, sensor/power/FRU/SEL rollup with health badge.
* Wired as a 'Hardware' tab in BOTH node UIs (corporate CorporateNodeDetailView +
default NodeModal) for parity. 21 i18n keys across all 7 languages.
Hardening from the adversarial review (3 confirmed findings, all fixed):
* MEDIUM — config-restore could enable hardware monitoring WITHOUT the versioned
acknowledgement audit record (non-repudiation bypass): restore now drops the
hardware_monitoring key and preserves the live consent state, so consent only ever
moves through the audit-logged set_hw_monitoring_consent path (settings.py).
* LOW — a non-int stored/posted ack_version raised int() -> 500 on every gate; add
_hw_int() so the gate fails CLOSED (disabled) instead of crashing.
* NIT — a crafted {"nodes":[123]}/[{...}] blew up _reject_bad_node's regex / set()
-> 500; validate the raw list + reject non-strings centrally -> clean 400.
Tests: +9 integration cases (install gates admin/consent/proxmox/bad-node + the 3
hardening regressions). Frontend build clean (Babel). Full suite 307 passed.
Step 1-2 of the #609 in-band hardware-monitoring phase:
* GET /api/clusters/<cid>/nodes/<node>/hardware (node.view) — serves the
parsed in-band sensor/power/FRU/SEL rollup from pegaprox/core/bmc.py, but
refuses with CONSENT_REQUIRED (403) until the feature is enabled.
* GET /api/hardware-monitoring/consent (node.view) — status + the versioned
compliance warning the UI must present.
* POST /api/hardware-monitoring/consent (admin.settings) — enable/disable.
Enabling requires acknowledge=true AND ack_version == the current warning
version (stale-UI opt-in protection); the acknowledgement is persisted to
the audit log with who/when/which-version, so opt-in is non-repudiable.
Warning text + version live in bmc.py (single source) so the audit record can
reference an exact version; bumping the version re-prompts.
Tests: 11 full-stack integration cases (consent gate 403->200, admin-only
enable, version-mismatch 400, cross-tenant 403, bad-node 400, audit-record
assertion, disable round-trip). Suite 298 passed.
Credential-free hardware health via local ipmitool on the node (SSH, in-band
over /dev/ipmi0 — no BMC network credentials, no data<->management-plane bridge).
Read-only: sdr/sel/fru/dcmi/chassis only, one SSH round via markers, graceful
when ipmitool or the IPMI interface is absent (never mutates the host).
Pure parsers (sensors/chassis/power/fru/sel) + a health rollup, fixture-tested
without hardware (10 tests). API route + mandatory compliance-ack consent gate +
audit-log + node Hardware panel follow; the network-Redfish opt-in is a later,
warned phase.
CodeAnt re-scan hunt (adversarially verified) surfaced a large second wave of missing tenant
gates + a priv-esc the first static_files fix missed:
- static_files.set_user_perms TENANT branch: a non-global-admin could set role='admin' (stored
unvalidated) or admin.* perms in their OWN tenant_permissions, which resolve to the target's
EFFECTIVE GLOBAL perms (has_permission runs with no tenant_id) => tenant->global priv-esc.
Now rejected for non-global-admins.
- new helpers.check_vmware_access (mirrors check_pbs_access) + applied to 21 vmware.py read/
action routes (get_vmware_vms/detail/hosts/datastores/networks/clusters/drs/ha/... ) that had
only a role perm and never scoped to the server's linked_clusters => cross-tenant ESXi BOLA.
- 14 more PBS routes gated with check_pbs_access (syslog/rrd/network/dns/time/health/forecast/
subscription/traffic-control/notifications) — the first sweep missed this route set.
- 13 cluster-scoped routes across pbs/users/clusters/settings/nodes now enforce check_cluster_access
(run_backup_job_now, get_security_audit, rotate_cluster_api_token, reconfigure/export/update
cluster config, rolling-update cancel/resume/clear, get_node_dns) — were role-perm only.
Regression tests: VMware cross-tenant route => 403; tenant-admin admin-role/admin.* amplification
=> 403 (both vectors). 279 passing. Remaining re-scan items (power/drift/schedules IDOR, app.py
http-splitting/CSRF/CSP, 2 SSRF, SIEM secret masking) in follow-up commits.
Second CodeAnt exploitation scan (on the current tip) surfaced 3 critical auth findings, each
verified real and fixed:
- static_files.set_user_perms: the GLOBAL-permissions branch (no tenant_id) only required
admin.users — which a tenant-scoped admin can hold — so a tenant admin could set a user's
GLOBAL permissions and grant themselves/anyone global-admin-equivalent perms (priv-esc). The
per-tenant branch already gated non-global-admins; the global branch now does too (requires
session role == ROLE_ADMIN).
- push._alert_handler: the cross-tenant scope I added last cycle FAILED OPEN when a subscriber's
user record was missing/deleted (still in push_subscriptions) — get_user_clusters({}) => None
=> treated as all-cluster admin => received every tenant's alerts. Now fails CLOSED on a
missing record (and on lookup error). Good catch by the re-scan on my own fix.
- rbac.user_can_access_vmware_vm: the role-permission fallback had NO tenant isolation (the
Proxmox user_can_access_vm has the equivalent guard) — any vmware.vm.* holder could reach
every VMware server's VMs cross-tenant. Now gated by the server's linked_clusters
(admin/unlinked open, else require get_user_clusters overlap), mirroring check_pbs_access.
Regression tests: tenant-admin global-perm PUT => 403 (global admin => 200); ghost/deleted
subscriber gets no cluster alert; VMware cross-tenant => denied, admin/unlinked => allowed.
274 passing. (The ~40 locked re-scan findings are under a parallel adversarial hunt.)
Drives POST /api/templates/custom with a cloud-metadata URL and asserts the SSRF guard rejects
it (400) — closes the harness gap for the SSRF fix (was only static + url_security-unit verified).
CodeAnt exploitation finding (incomplete session handling): update_user set a new password hash
but never killed the user's live sessions, so a stolen/old session survived the reset. Now the
password branch calls invalidate_all_user_sessions() (mirrors admin_change_password), scoped so
role/email/tenant edits don't needlessly log the user out. Regression tests: password reset kills
the target's session (401); a non-password edit does not.
CodeAnt exploitation findings (IDOR, high→low) — cross-tenant reads/actions with only a role perm:
- dr_drill.py: start_drill / get_drill / list_drills never checked the plan's clusters, so any
site_recovery holder could trigger a drill on, or read the drill history of, another tenant's
plan. Added _require_plan_access (check_cluster_access on the plan's source+target cluster);
reads return 404 when denied (don't confirm existence).
- history.py: get_scheduled_tasks, get_migration_history, get_affinity_rules returned every
tenant's rows to any cluster.view holder — now filtered by get_user_clusters (+ the
path-scoped affinity alias enforces check_cluster_access).
- alerts.py: get_alerts exposed every tenant's alert rules — now filtered by get_user_clusters.
All use g.current_user (set by require_auth) => None for admin/default-tenant (unchanged), else
scoped. Regression tests: DR-drill start + list cross-tenant => 403/404, owner => 200. Full
suite 267 green.
CodeAnt exploitation finding (IDOR, high→low): ~20 PBS routes reached pbs_managers[pbs_id]
with only a role permission and never called check_pbs_access — a cross-tenant BOLA. The
headline cases (download_pbs_file, browse_pbs_catalog) let a non-admin read/list ANOTHER
tenant's backup file contents + catalog tree; list_pbs_servers returned every PBS unfiltered,
making enumeration trivial. All 30 sibling routes already gated with check_pbs_access — these
were the gaps.
Fixed: check_pbs_access(pbs_id) guard added to download_pbs_file, browse_pbs_catalog,
get_pbs_tasks, get_pbs_task_detail, get_pbs_jobs, get_pbs_datastore_config, get_pbs_*_notes,
get_pbs_namespaces, get_pbs_datastore_rrd, get_pbs_disks, get_pbs_disk_smart, the 3 report
routes, get_pbs_remotes, diff_pbs_backups, and the apt routes (20 total). list_pbs_servers now
filters both the connected + disabled-DB listings to PBS the caller can reach (admin / unlinked
/ tenant-overlap), mirroring check_pbs_access semantics.
Regression tests (harness, real app): cross-tenant tasks + catalog-download => 403; listing
filtered per tenant; admin sees all. 264 passing (was 260).
Three critical/high auth-bypass findings from a CodeAnt AI-exploitation scan, each
independently verified as REAL (adversarially cross-checked + reproduced) and fixed:
- auth.py / users.py — off-boarding bypass: require_auth did get_user() or {}, so a
DELETED user resolved to {} -> {}.get('enabled', True)==True passed the disabled
check and the role fell back to the stale session role; get_user_clusters({})==None
= all-cluster read. delete_user's inline session purge was also DEAD (operated on a
stale active_sessions binding after load_sessions rebind) and never revoked pgx_ API
tokens. Fix: require_auth fails CLOSED when the record is gone (ACCOUNT_DELETED,
covers session AND token auth); delete_user uses invalidate_all_user_sessions() +
revokes api_tokens.
- push.py — cross-tenant alert leak: _alert_handler wrote every cluster alert (cluster/
node/VM names + live metric %) into EVERY push subscriber's inbox and woke them, with
zero tenant scoping. Fix: scope recipients to subscribers who can reach
alert_data['cluster_id'] via get_user_clusters (admin/default-tenant still get all;
cluster-less system alerts go to all; fail closed on lookup error).
- groups.py — cross-tenant cluster hijack: assign_cluster_to_group / rename_cluster only
gated the source cluster with check_cluster_access, whose ACL/pool fallbacks pass on
mere VM-level reach; a non-admin with admin.groups + one foreign VM-ACL grant could
move that cluster into their own group and gain cluster-wide tenant membership. Fix:
require real tenant ownership (get_user_clusters(include_pools=False)) of the source
cluster before re-grouping/renaming.
Regression tests (integration harness): test_integration_auth_deleted / _groups / _push
drive each through the real app. 260 passing (was 249).
Phase 3 breadth — per-blueprint integration tests driving real requests through
the real Flask app against the proven harness (conftest _integration_app + api /
seed fixtures). Every route is exercised end-to-end: require_auth -> perm gate ->
check_cluster_access -> the 404 gate -> _require_vm_access -> the (faked) manager.
vms (38) — per-VM lifecycle/detail: config, status/start/stop/reboot,
snapshots-list, clone, migrate + remote-migrate, resize/move
disk, unlock, guest-file-read. Cross-tenant + pool-reach-only
denies AND admin allow-paths.
clusters (29) — the RESTRICTIVE /resources filter (vs the additive per-VM
routes), status reads, admin-only add/edit/delete + HA, both
cluster-level and perm-gate denies.
storage (25) — list/content/create/edit/delete + the SSRF-gated
download_from_url (metadata/loopback/private rejected through
the route; public IP-literal reaches the manager).
nodes (28) — node reads (no _require_vm_access -> pool-reach user allowed,
the key nodes-vs-vms difference), node.shell / node.network
perm gates, the _reject_bad_node defense, HA priority.
snapshots (21) — per-VM snapshot list/create/delete/rollback: cross-tenant +
pool-reach denies + admin allow.
users_acl (25) — user CRUD + vm_acl get/set/delete (admin / user-management
gated, non-admin 403; admin ACL set round-trips).
Verified: 249/249 in a single process (no cross-file session/manager/DB leakage);
each suite adversarially reviewed for vacuous/false-green asserts (none: every deny
asserts the specific 401/403 + manager-not-called, every allow asserts a real body
property). MUTATION-CHECKED: neutralising _require_vm_access turns exactly the 8
VM-level deny tests red and nothing else — the suite has teeth against a real authz
regression, the exact class the direct RBAC tests structurally miss.
Phase 3 of the test-suite roadmap. The RBAC tests exercise user_can_access_vm()
directly; these fixtures drive REAL requests through the REAL Flask app + real
blueprints so authz is asserted end-to-end — the layer the 2026-07-12 'BOLA'
mis-framing proved the direct tests can't cover.
Harness (tests/conftest.py):
- session-scoped _integration_app: create_app() once, against an isolated temp DB
so it never touches the developer's real encrypted DB (which has live clusters);
save_sessions() silenced.
- per-test isolation via the existing db fixture (routes call get_db() lazily, so
re-pointing the db globals per test gives a clean DB while the app lives on).
- auth via the REAL server-side session store (create_session -> active_sessions,
X-Session-ID header) — the production login path, not a forged cookie.
- managers faked + injected into pegaprox.globals.cluster_managers; the deny path
(403) fires before the manager is touched, so deny-tests need no method stubs.
- _ApiClient bakes in the session header + (for writes) the same-origin/XHR proof
the CSRF gate requires. make_fake_manager() builds a per-test MagicMock manager.
Smoke (tests/test_integration_smoke.py, 6 cases, green): anon 401; admin reads
config 200 (manager reached); pool-reach user denied at the VM level 403 (manager
proven never called); cross-tenant user denied at the cluster level 403; authed
write without CSRF headers blocked; harness write path passes CSRF -> 404 routing.
83 passing locally (was 77).
Phase 1 of the test-suite roadmap — executable invariants for the three input
guards the static reviewers can only read, not exercise:
- test_sanitization.py (32 cases): the shell-sink validators
(validate_content_filename / _esxi_path_component / _storage_name) reject the
quote-breakout, path-traversal, space and shell-metachar payloads that map into
the SSH / qm / pvesm / qemu-img pipelines; the sanitize_bool coercer pins the
'false'-string-is-not-truthy fix.
- test_url_security.py (SSRF): private/loopback/metadata IP literals + dangerous
schemes + CR/LF/NUL blocked with no DNS; DNS path monkeypatched (_resolve_all)
so it's deterministic and offline. Pins the pentest fix: require_resolution=True
=> an unresolvable host fails CLOSED (the old hand-rolled guard failed open).
- test_csrf_origin.py: drives the real app.py validate_request gate through the
Flask test client — suffix-confusion, userinfo, protocol-relative, tab-in-scheme
and wrong-port Origins are blocked even with the XHR marker; same-origin passes.
77 passing locally (21 existing + 56 new), no regressions.
The static reviewers (CodeAnt/Aikido/Qodo) read the code but never EXECUTE the
invariants, and pytest had no CI trigger at all (only docker/release workflows).
- .github/workflows/test.yml: runs the suite on pull_request + pushes to main/Testing
(ubuntu-24.04 = Python 3.12, SHA-pinned checkout, read-only token, persist-credentials
left at default per Nico).
- tests/test_route_authz_contract.py: enumerates every /clusters/.../<vmid>/... route
from the live url_map and asserts each handler references a per-VM authz gate, or is
a documented _EXEMPT. A new VM route without a gate — or a removed gate — turns CI
red. It immediately caught the remote-migrate IDOR gap (fixed in 2548104). 21/21 green.
- conftest closes the real threadlocal handle (self._local.conn), not the
always-None self._conn, so the temp DB file isn't held open on rmtree.
- README + test_authz_isolation: the inherit_role BOLA bug is fixed, so drop
the xfail-marker documentation and the now-unused 'import pytest'.
The vm_acls table never had an inherit_role column, so save_vm_acl and the
legacy-import path silently dropped it and get_all_vm_acls could not restore
it — user_can_access_vm read vm_acl.get('inherit_role', True) and always got
True. Effect: an ACL saved via the UI's 'custom permissions' mode
(inherit_role=False, e.g. 'this user may only view+console this VM') granted
the user the FULL vm.* permission set instead. Broken access control.
Fix: add an inherit_role column to vm_acls (CREATE + idempotent ALTER-TABLE
migration for existing DBs, defaulting legacy rows to 1/True to preserve the
historical effective behaviour), write it in save_vm_acl and the JSON->SQLite
import path, and read it back (default True) in get_all_vm_acls.
Caught by the new authz regression suite; the previously-xfailed
test_vm_acl_inherit_role_false_restricts_to_listed_perms now passes. Migration
verified end-to-end (add-once, idempotent, legacy=True, inherit_role=False
persists).
First automated coverage for the RBAC layer (pegaprox/utils/rbac.py) — the
highest-risk security surface in a multi-tenant cluster manager and the one
static scanners (Aikido/CodeAnt) structurally can't see. 17 in-process tests
(no live cluster needed) lock in the BOLA / tenant-isolation invariants that
were historically fixed only by hand review (#490/#493/#495/#555):
- cross-tenant VM access denied; cluster list scoped to tenant
- VM-ACL additive-not-restrictive model; #555 pool/ACL cluster-reach guard
- API-token privilege floor (min(token, owner)); no admin-bypass for an
admin-owned viewer token; mint-ceiling
- pool.admin vs granular pool perms; pool grant doesn't leak to non-member
VMs or across clusters
Harness: throwaway encrypted DB per test (temp dir + both DB singletons +
rbac process-caches reset → order-independent). pytest is dev-only
(requirements-dev.txt), not shipped in the appliance.
The suite already caught one real broken-access-control bug: the vm_acls table
has no inherit_role column, so an ACL saved with inherit_role=False (the UI's
'custom permissions' mode) is silently stored as FULL VM access. That test is
marked xfail(strict) with the root-cause + fix pointer until it's fixed.
