JAWA v3.2.0 - #71
Open
Chris Ball (ball42) wants to merge 90 commits into
Open
Conversation
Type hints, code formatting, utility functions -> develop branch for further testing
refactor: reducing complexity, introduced constants, adjusted success message for custom webhooks (resolving #53)
* chore: update .gitignore to exclude JAWA runtime data files * feat: add template catalog and enable/import functionality
* feat: add shared infrastructure modules * feat: implement automation handlers for various services * feat: add unified automation templates and views
* feat: consolidate CSS, update base layout * feat: update existing templates for new design system
* feat: implement backward compatibility redirects for legacy webhook routes * feat: update layout and styles for improved UI * chore: update .gitignore
* feat: add CI pipeline, test fixtures, and smoke tests (#62) - Add GitHub Actions CI workflow with Python version matrix. - Define shared pytest fixtures for isolated environments and Jamf API mocks. - Introduce initial smoke tests for blueprints, webhooks, and login flows. - Configure ruff and pytest; update dev dependencies. * chore: specify version for ruff in requirements-dev.txt
…7) (#63) * fix: receiver tolerates missing webhook auth keys (B1) * fix: route template webhooks through data_store with canonical auth shape (B1) * fix: reject path-traversal script filenames in template import (B7) * test: fix mis-targeted traversal assertion in template import test (B7) * fix: store absolute script paths for template webhooks so they execute (B1)
#64) * feat: add session-timeout ladder and fail-safe resolver (J6) * fix: enforce session lifetime and harden session cookies (B4) * feat: persist admin-configured session timeout at /setup (J6) * feat: add session-timeout dropdown with extended-tier warnings to setup (J6) * feat: drive session-timeout warning modal from configured value (J6) The client-side timeout warning modal now counts down against the effective server-resolved timeout instead of a hardcoded 15 minutes, so it never advertises a window longer than the server enforces. inject_common_vars now exposes session_timeout_seconds (deferred import of _resolve_session_timeout to avoid a circular import), and the modal JS reads it with a default(900) guard. Also wires the /setup timeout help note to its select via aria-describedby. * fix: get_server_config returns {} for non-dict server.json (J6 hardening)
… (B2/B3/B5 + J6 follow-up) (#65) * fix: allow opting out of Secure cookies for local http dev (J6 follow-up) SESSION_COOKIE_SECURE was hardcoded True in the J6 work, which is correct for production (HTTPS behind nginx) but breaks local 'python3 app.py' runs over http: the browser drops the Secure session cookie, so login succeeds server-side but the session never returns, producing a login loop. Default stays Secure; set JAWA_INSECURE_COOKIES=1 for local http development only. HttpOnly and SameSite are unchanged. * fix: guard webhook receiver payload parsing and method check (B2) * fix: handle empty/missing file selection in resource deletion (B3) * feat: add branded 500/403/405 error handlers (B5) * fix: surface login errors in all home render branches * feat: retain username and JPS URL on failed login * fix: only retain login fields after a failed login (prevent prefill phishing) * fix: retain login fields via one-shot session flash, not query params (anti-phishing)
Creating a jamfpro/okta automation before JAWA is configured raised a
'Setup Required' error page that dead-ended the user -- Back and
Dashboard only, no path to /setup where they actually need to go.
AutomationError already carried an optional link, but the two raise
sites passed none and the template rendered a link as a raw URL in a
new tab.
Now the raise sites pass link=/setup with a friendly label, and when a
link is present the error page renders it as the primary in-page action
button ('Go to Setup'), demoting Dashboard to secondary. Adds link_text
to AutomationError for the button label.
) The warning modal counted down a setInterval variable, which the browser pauses/throttles during sleep or when the tab is backgrounded. After a long idle the countdown never advanced, so no warning fired and the server session had already expired -- the user just got a silent bounce to login on their next click. Track an absolute expiry timestamp instead and re-evaluate against the real clock on tab focus/visibilitychange, not only on the interval. On return: if already expired, redirect to /logout with a 'Session expired' reason so the login page explains why (reusing the login-error banner); if within the warning window, show the modal immediately with the correct remaining seconds.
…ts, okta stub) (#68) - Drop unused mongoengine dependency (never imported anywhere). - Delete bin/load_home.py (dead duplicate; the live load_home is in views/home_view.py, and the dead file had a broken import). - Remove the commented-out legacy blueprint imports in register_blueprints. - Remove the empty okta_verification.main() stub and its __main__ guard; verify_new_webhook (the real function) is retained. No behavior change. App boots, ruff clean, full harness green.
…ard, error swap (#69) * fix: cap upload size at 16MB (Flask MAX_CONTENT_LENGTH + JAWA nginx vhost) * fix: strip trailing slashes from setup URLs (prevents double-slash webhook URLs) * fix: reject script uploads without a shebang (fails clearly at upload, not trigger time) * fix: setup submit button reads 'Save' when editing an existing config * test: lock in that login fails against a non-Jamf URL (3.0.2 auth regression guard) * fix: /error route no longer swaps title and message The error() view passed render_template kwargs crossed (error_message=error_title, error=error_message), so the branded page showed the title as the body and the message as the h1. Now aligned with how error.html and _error_page use them: error = title, error_message = body. Test asserts each lands in its correct element (has teeth: fails if the swap returns).
The legacy /webhooks/* and /cron/* compatibility routes interpolate a user-controlled value (name/target_job/target_webhook) straight into a 301 redirect path. A value like //evil.com or /\evil.com makes the result protocol-relative, so the browser resolves it off-site (open redirect / phishing; CodeQL py/url-redirection). Add bin/url_safety.safe_path_segment (no third-party imports, no cycle risk) that strips slashes/backslashes from single-segment names, and apply it to all five app.py interpolated redirects. The template_view /workflows/<path:rest> catch-all guards against backslash/'//'/scheme tails, falling back to the catalog. Tests cover off-site payloads per route plus the normal-value happy paths.
The form listed 23 events alphabetically in a hardcoded block. It now renders optgroups from the same event catalog the Webhook Reference pages read, so the two lists cannot drift, and computer, mobile device and system events are visibly separated when picking an event. Values are unchanged Jamf event strings. A stored event the catalog does not list is rendered in its own group, so opening and saving an older automation cannot silently change its event.
Flattening the catalog with sum(start=[]) raised a TypeError if a
hand edit left a category holding a bare string or a mapping instead
of a list, which took out both the create and the edit form. The
Webhook Reference overview survives the same damage, and the catalog
reader degrades rather than raising for exactly this reason, so the
form should too.
Categories are now filtered to list-shaped values before they are
flattened and looped, so a single malformed group drops out and the
remaining events still render. Also records what the `or {}` fallback
actually guards: the partial being rendered without the context key,
not a damaged catalog file.
The degradation guard in _jamf_fields.html has two rejectattr filters (one for strings, one for mappings) to avoid TypeError from the later flatten. The existing test only verified the string case. Parametrizing over both damage shapes ensures the mapping half is also exercised.
Three assertions in the webhook reference and event dropdown tests could not detect the defect they named. The overview tests asserted only strings the sidebar also emits -- category headings, event names, and detail links -- so both passed with the entire overview table deleted. They now also assert an event description and the standard table class, neither of which the sidebar renders. Two tests counted events in the catalog rather than on the page: len(events) == 23 reads the file the fixture just copied, so no rendering defect can move it, and a 24th Jamf event would fail it spuriously. Both now compare the rendered event list against the catalog, so a dropped, duplicated, or spurious entry fails and growing the catalog does not. The required-placeholder test never asserted required. A bare containment check cannot discriminate -- the upload macro and the name field emit it too -- so the assertion is scoped to the event select's own opening tag. Without it the create form accepts a webhook with no event, which then matches no inbound webhookEvent: a webhook that exists and never fires. Its absence in edit mode is asserted alongside the "keep current" placeholder that depends on it.
The event dropdown already drops a category a hand edit left holding a bare string or a mapping instead of a list of event names. The webhook reference read the same catalog with no such guard. Neither shape crashes the page, which is why it went unnoticed: Jinja iterates a string character by character and a mapping over its keys, so a miscategorised entry renders a table row and a sidebar link per character or per key, each linking to an event that does not exist and answering 404. Filtering once at the top of the template and looping the filtered groups in both the sidebar and the overview table puts all three consumers of the catalog under one shape rule. The guard belongs in the template rather than the accessor: three consumers read that dict, and coercing it centrally would silently reshape hand-maintained reference data. Also merges the duplicated .copy-btn:focus-visible rule, which declared opacity in one block and the focus ring fourteen lines later.
The README already announced 3.2, but four runtime sites still reported 3.1.1. The two User-Agent headers are the ones that mattered: they are what a JAWA instance reports to Jamf Pro, so a shipped 3.2 would have identified itself as 3.1.1 in customer Jamf logs. - views/_type_handlers/jamf_handler.py: USER_AGENT_STRING (3 call sites) - views/home_view.py: the activationcode probe's User-Agent - templates/shared/_layout.html: console footer - bin/installer.sh: install banner (same-length swap, art alignment kept) The README v3.1.1 heading stays: it is that release's historical notes. Version is still hardcoded in four places and the User-Agent literal is duplicated across two files; consolidating onto one constant is left as a follow-up rather than folded into a release-prep bump.
Same inert-ignore bug 52f0520 fixed for webhooks.json: data/cron.json was listed in .gitignore but still tracked, so the rule never applied and local timed-automation state showed up as a diff on every checkout. The committed copy also meant a checkout could overwrite an operator's real cron definitions with the empty seed. data_store._read_json recreates it lazily as [] on first read, so a fresh clone is unaffected. data/time.json stays tracked: it is static seed data (day/hour/frequency lookup tables), not runtime state.
enrollment_pipeline.py shipped as a 6-stage outline with 7 undefined names and needed a device-assignment CSV contract that exists nowhere in the repo; cut it rather than invent one. smart_group_appletv.py had no Config and called an undefined perform_api_call. All four Jamf-calling scripts now share one byte-identical canonical API block. ruff.toml excludes this directory, so a new pytest guard applies F821/F401 to the bundled scripts instead.
…16, B15)
ea-update targeted ComputerInventoryCompleted while its script PUTs to
a mobile-devices endpoint; teams-notification declared the non-event
"Any Event" (now null = any). Both smart-group schemas set
event.computer to a boolean, so the event.get("computer", {}).get()
fallback raised AttributeError -- replaced with a type-checking
_device_field helper.
config_params gain an explicit token, separating the replace-needle
from the UI hint (COOLDOWN_HOURS's old needle "12" matched twice in
its own script). Entries gain hook_name: a legal single-string Jamf
webhook name, required because Jamf rejects names with spaces and all
eight titles had them. Dead notebook_slug removed; exit codes
reconciled against the scripts.
A null trigger_event renders as the literal "None" in Jinja and would
be persisted as the webhook's event, so the three template surfaces
and the stored value now fall back to "Any Event" / "".
_apply_form_params ran markupsafe.escape() over form input and then string-replaced it into PYTHON SOURCE, so any value containing & " ' < > was baked in mangled -- Power Automate and Logic Apps webhook URLs are entirely & runs. _apply_credentials did not escape, so a quote in a saved secret produced invalid Python instead. Two paths, opposite bugs, one feature. Both are replaced by one substitute_params that injects real Python literals via repr(), validates numerics, and fails loud on any unfilled field rather than baking the placeholder into the deployed script.
substitute_params applied one str.replace per param against a shared
accumulator, so text an earlier param had already substituted was
re-scanned by every later param's replace. A value carrying a later
param's needle was therefore rewritten from the inside, breaking out of
the Python string literal it was meant to be trapped in:
server_url = x"__JAWA_CLIENT_ID__"y
client_id = +__import__("os").system("...")+
-> self.server_url = 'x'+__import__("os").system("...")+'y'
Enable returned 302, the webhook registered, and the payload ran when
the receiver instantiated Config() on the first fire. Affects every
multi-param workflow. Substitution is now a single re.sub over an
alternation of the tokens, so emitted text is never reconsidered, plus
a check rejecting any replacement that contains a __JAWA_ needle and
one refusing any token that survives.
Also tightened the two validators that let a script through to fail at
trigger time instead of at enable: the numeric branch accepted inf,
nan, 1e400 and past-limit integers, which repr() to bare words that are
not builtins (NameError on first fire), and unicode digits that mean a
different number than the glyphs typed; the raw branch was a character
blocklist that leaked NUL (deployed .py would not compile), CR (silently
rewritten to LF by the XML parser) and the other C0/C1 controls, now
rejected by range rather than by list.
_validate_package checked field presence and filename safety but never that the script parses, so a truncated or malformed .jawa.json was written, chmod 0755'd and registered as a live webhook -- failing later inside Popen where the only signal is a logged non-zero exit code. A compile() gate rejects it with the offending line number before anything is written. Deliberately a parse gate only: undefined-name analysis is right for bundled content, where the canonical API block is the contract, but would wrongly reject a user script that relies on a runtime global. Also folds in a fix a reviewer found in the Task 3 substitution engine: substitute_params returned early when a workflow declared no config_params, skipping the TOKEN_PREFIX survivor check, so a template carrying an undeclared __JAWA_* token shipped with the placeholder baked into the deployed script -- exactly the drift that check exists to catch. Latent only today (all seven bundled templates declare at least one config_param), now guarded by a regression test.
…amf (J16, B14) Template enable wrote tag "custom" while carrying a Jamf event, so every enabled template was misfiled under Custom and edited against CustomHandler's form -- which has no event field, making the trigger that drives the automation invisible and uneditable. Nothing was created in Jamf Pro either, with no indication the user had to build that side by hand. Enable now creates the webhook in Jamf Pro first and only writes locally once Jamf accepts, so a 409 or timeout leaves no orphaned script. Entries carry tag "jamfpro", the real event, and jamf_id, so they file correctly and "Open in Jamf Pro" works. The enable form gains Basic/header auth and, for any-event templates, an event picker sourced from the same catalog the create form uses. Catalog entries carry an explicit hook_name because Jamf rejects webhook names containing a space and all the template titles had them. Also XML-escapes the interpolated name and event in _build_webhook_xml -- pre-existing on the jamfpro path, but templates newly feed user-supplied names through it. Enable also reuses the create path's one-shot success flash rather than a bare success_msg. Jamf creates a smart-group webhook DISABLED, and three bundled templates use a smart-group event, so reporting a plain "Enabled template" for those left the user believing the automation was already live -- the same silent-extra-step failure this bug is about.
…B14 review round 1) Review fixes on top of d5ba4bc. _extract_auth_fields used form.get(key, "null"), but a get() default only fires when the key is ABSENT. The auth fields are labelled optional, so picking Basic or Header auth and leaving the boxes blank posts "" -- which was then stored verbatim while _build_auth_xml told Jamf NONE. The receiver defaults an unauthenticated request to the string "null", so "null" != "" made validate_webhook reject every inbound event, permanently, behind a success page that said "Enabled". Reproduced as a 401 before the fix. Also in this round: - Template config params are now marked required only when a saved credential set cannot supply them. Selecting a credential set fills server_url/client_id/client_secret server-side and nothing populates them in the browser, so requiring them blocked submit until the admin retyped the client secret. CREDENTIAL_KEYS is passed into the template rather than duplicated in Jinja. - Webhook-name validation is one shared rule (validate_webhook_name) used by both the create path and the template path, and is a positive character rule instead of a blocklist. xml_escape now makes "Dev&Prod" well-formed XML, so Jamf would accept it and then call /hooks/Dev&Prod -- which Flask routes to "Dev" with "Prod" as a query param, silently never firing. The old blocklist also missed "#", "?", "%" and tabs. - The stored "enabled" flag mirrors what Jamf actually did: a smart-group event is created with enablement "false", so a flat True made the local record contradict the remote object. - The success flash reuses automation_view._flash_success instead of a near-copy with a divergent filter. - Four inline styles in enable.html moved to token-based CSS classes. Tests: adds the anti-drift coverage for the event picker, the prefilled hook_name, and the zero-POST ordering guarantee (a config JAWA refuses must not leave an orphan webhook in the customer's Jamf Pro), plus the blank-auth regression above and both sides of the enabled flag. Replaces a try/except that would have let a 500 pass as success with an assertion that the redirect lands on /error.
The bundled scripts stay self-contained so a user can download one and
run it standalone, which means the Jamf API block is duplicated rather
than imported. That is only safe while the copies stay byte-identical,
so assert it.
Also pins the shebang and the executable bit, as two separate
invariants rather than one. The shebang is the load-bearing half: this
directory is the shipped template SOURCE, and enable copies its content
verbatim into the deployed scripts dir, which the receiver execs with a
bare Popen and no interpreter prefix -- so a template missing its
shebang yields a deployed script the kernel cannot start. The exec bit
on the source is not what the receiver depends on, since the deploy
step chmods its own output; it is asserted for the standalone-download
promise, and the receiver-facing invariant now has its own test that
the deployed copy really is 0o755.
The B15 payload replay was vacuous for the script that motivated it.
Pinned to its own trigger_event, event-tracker-sqlite only ever saw
ComputerCheckIn, whose example carries no "computer" key at all, so the
nested-dict branch never ran and the boolean case -- the actual bug --
went untested. The replay is now every example payload against every
script, which is also the honest contract: the trigger lives in Jamf
Pro, where an admin can point any event at any JAWA webhook. A
companion test asserts some example still sets event.computer to a
bool, so the replay cannot go quietly vacuous again.
_device_field additionally tolerates a non-dict event body in both
scripts that define it. Nothing Jamf sends looks like that and both
call sites already do event_data.get("event", {}), so this is hardening
rather than a fix -- but the body is JSON off the wire and these
scripts run unattended, where a TypeError is just a non-zero exit code
in a log rather than anything a user sees.
Finally, pytest.ini silences one environmental import-time warning from
requests about its urllib3/chardet version ranges, matched on the
message so resolving it does not require importing the module that
emits it, and so a different version mismatch still surfaces. The full
suite now runs clean with no warnings.
Enable creates the webhook in Jamf Pro; import did not. Same catalog, same kind of package, opposite behaviour -- and an imported package is predicated on a Jamf Pro event just as much as a bundled one, so leaving that side to be built by hand is the silent-extra-step failure B14 exists to close. The import form gains a "Create webhook in Jamf Pro?" checkbox, checked by default, and reads it as absent-means-cleared since an unchecked box posts nothing. Checked reuses the enable path's machinery rather than reimplementing it: the same name validation, the same duplicate check, the same smart-group enablement, the same auth helpers, and the same fail-closed order -- Jamf is asked first, so a 409 leaves no orphaned script on disk and no half-configured automation. The entry lands with tag "jamfpro", the real event, and jamf_id, and the success page deep links the new object. Auth goes through _build_auth_xml and _extract_auth_fields even though this form carries no auth fields yet: both reduce to unauthenticated today, but deriving the XML JAWA sends and the credentials JAWA stores from one place is what stops them drifting into telling Jamf NONE while storing something the receiver then rejects. Cleared keeps today's behaviour exactly -- tag "custom", no jamf_id, nothing created in the customer's Jamf Pro. Name validation is re-framed on the way out because this form has no name field. The name comes from the package, so the create form's "rename it" advice is not something the admin can act on here; the message names the package file and points at the checkbox instead. Both paths now use the one-shot success flash, so import stops being the last route that redirected with the message in the query string. Tests cover both branches, the fail-closed ordering, the rejected name, the smart-group not-yet-live warning, and -- the invariant this module exists for -- that a Jamf-registered import actually fires through the receiver. Each was mutation-tested: seven mutations, each caught by the intended test.
… system Two problems on one page, kept in one commit because the tests covering them do not split cleanly. Destructive-action safety: Download and Delete sat flush against each other in one flex row, same size, so a misclick on a benign action landed on a destructive one. Add a gap, and route Delete through the shared delete_confirmation macro instead of the hand-rolled copy of that card the page carried -- style block and all -- which is how the two drift. The macro grows two optional arguments rather than this page growing a second confirmation screen: - cancel_url, because a blind history.back() walks to the spent list form (the same reason the success pages stopped using it). - warning_detail, because the shared line promises to delete "all associated data", which describes an automation and not a file. The real consequence for a file is a script that starts failing. Both default to today's behaviour, so the automations delete page is unchanged. Design system: the page predated the system pass and kept its own table, headings and layout. Move the listing onto .hippocrates inside an overflow wrapper, replace the centred h4s with section-headers, demote the resources path from a navy+mono code block to a caption (that treatment is for code and logs, and a directory path is neither), make the whole row a hit target for its radio, and give the empty case an empty_state that invites an upload instead of a bare "No files uploaded". Add Size and Type columns while the view is open: both are what an admin needs to decide what to download or delete, and sizes are formatted so a 412-byte script reads "412 B" rather than "0.0 KB". Two bugs surfaced while listing the directory: - Hidden files were filtered by removing from the list being iterated, which shifts the next element past the cursor -- so a dotfile immediately following another leaked into the page. Filter by comprehension, and sort, so the order is not left to the filesystem. - listdir-then-stat is a race. A file deleted by a second admin mid-request took the whole listing down with a 500; skip it instead.
The receiver's unauthorized branch interpolated webhook_name -- which comes straight off the /hooks/<webhook_name> path -- into a bare-string return. Flask serves a bare string as text/html, so a POST to /hooks/<markup> with bad auth reflected unescaped markup back onto JAWA's own origin. There is no CSP and no after_request escaping to blunt it. The two sibling returns in the same function were safe only by accident: they return dicts, which Flask serializes as JSON. Drop the interpolation rather than escaping it. The name is already on the warning log line above, which is where an operator debugging a 401 looks, and a body carrying no request input cannot regress if the content type ever changes. The escape() pattern used in app.py would also have worked; not reflecting at all is the stronger guarantee. Add a regression test asserting a markup payload in the hook name never reaches the 401 body, verified failing against the previous code. This is the one live-file CodeQL py/reflective-xss alert that is real rather than a false positive; the others either sit in files this release deletes or return JSON.
The existing v3.2 section was written before the UI sweep and the templates work, so it described roughly half the release: it covered the harness/CI, session timeout, script docs, template firing, path traversal, resource deletion, error pages and dead-code removal, and mentioned nothing from the success-page fix, the dashboard/Extras polish, the Webhook Reference page, the Resource Files page, or the bundled-template content work -- which is the flagship. Adds an "Upgrade notes" block up front for the four things that change behaviour on an existing install rather than merely fixing it: - v3.2 is the last release carrying a v2 migration path. The v2 upgrade code is untouched here; this is the notice, not the removal. - Previously-enabled template webhooks start firing on upgrade, and template webhooks are unauthenticated by default. An operator who enabled one months ago and saw nothing happen needs to know it is about to become a live open endpoint. - Stricter webhook-name validation on create now refuses # and %. - Anything shipped inside data/ is not upgraded in place, because the installer restores the operator's data/ over the shipped copy. That covers the bundled template scripts and the webhook event catalog, so an upgrading operator keeps the versions they first installed. The rest is reorganised into New features / Bugfixes / Removed / Repository maintenance, following the existing entries' shape. Notes that DeviceRateLimited is listed with its sample payload pending rather than inventing one, and that the Enrollment Pipeline template was removed rather than shipped as an outline. Heading is v3.2.0 to match the tag and the three-part style of the v3.1.1 / v3.1.0 entries. The historical v3.1.1 heading stays as changelog history.
Reported against a real instance: login failed with "activationcode
response was not Jamf-shaped; refusing login" for an account that has
working API access. Querying /JSSResource/activationcode directly on the
same instance with the same account returns
{"license_information": {"organization_name": "...", "code": "..."}}
The guard required a top-level "activation_code" key, so it rejected
that. The result was a hard lockout of the console against a genuine
Jamf Pro server -- the product's core function -- with an error message
blaming the operator's URL.
Accept either wrapper key. A random website's JSON carries neither,
which is all this guard needs to distinguish; it is defense in depth
behind the token check, not the primary authentication.
The reason the harness did not catch this is the more important half:
tests/conftest.py mocked the activationcode response as
{"activation_code": {...}} -- a shape no instance sends. The fixture
invented the contract and every login test then verified the code
against that invention, so the guard looked covered while being wrong
about the one thing it inspects. The mock now returns the live shape,
which is why the old guard fails four fixture-based tests once reverted.
Adds three tests: the live "license_information" shape logs in, the
"activation_code" spelling logs in, and a 200 of valid JSON carrying
neither key is still refused. That last one is tighter than the
existing non-Jamf case, which only covered a body that fails to parse
as JSON at all. Mutation-tested: the license_information test fails
against the previous guard.
Five open Dependabot advisories, all medium, and the compatible-release pins were what blocked the fixes rather than merely lagging them: requests ~=2.31.0 allows <2.32.0, patched at 2.32.0 / 2.32.4 / 2.33.0 Werkzeug ~=3.0.1 allows <3.1.0, patched at 3.1.4 / 3.1.5 bin/installer.sh installs straight from requirements.txt, so shipping 3.2.0 unchanged would have put the vulnerable versions on every new install. Bumped to ~=2.33.0 and ~=3.1.5, resolving to requests 2.33.1 and Werkzeug 3.1.8 against the existing Flask ~=3.0.2. Werkzeug 3.1 changed one behaviour the suite depended on: test_workflows_rest_rejects_off_site asserted the Location header does not start with "http" as a stand-in for "not off-site". Werkzeug >= 3.1 emits an absolute Location for its own routing-layer canonicalization redirect (merging a "///" run), so a same-origin hop now arrives as "http://localhost/workflows/evil.com" and tripped a string-prefix check while remaining entirely local -- evil.com is a path segment there, not a host. The open-redirect protection itself never regressed. Rather than special-case the string, the assertions now parse the Location and compare its host against the request host, which is what off-site actually means. That also covers a case the prefix check could not: an absolute URL to a genuinely different host. Prefix and embedded-"//" checks now run against the parsed path, so a legitimate scheme's "//" no longer reads as a protocol-relative payload. Under 3.1 the "///" payload is merged at the routing layer before JAWA's view runs, so the test accepts either destination -- /templates from JAWA's own shim, or /workflows from Werkzeug's canonicalization -- and asserts same-origin on both. Verified on both generations: 415 passed under Werkzeug 3.0.6 with the old pins and under 3.1.8 with the new ones. Mutation-tested by neutering safe_path_segment, which fails 30 of the 43 redirect tests, so the origin comparison still catches a real off-site redirect.
CI caught what local testing could not: requests 2.33.x declares Requires-Python >=3.10, so the 3.9 leg of the matrix could not resolve requests~=2.33.0 at all and failed at dependency install. The 3.14 leg passed, which is exactly why this needed the matrix. Dropping 3.9 is a supported-platform decision about who can run JAWA, not a side effect of clearing an advisory, so the pin moves to ~=2.32.4 (resolves to 2.32.5) and 3.9 stays supported. Werkzeug 3.1.5 is unaffected -- it declares >=3.9. That clears two of the three requests advisories, both the ones that matter for how JAWA uses the library: GHSA-9wx4-h78v-vm56 Session does not verify later requests after a first request with verify=False (fixed 2.32.0) GHSA-9hjg-9r4m-mvj7 .netrc credential leak via malicious URLs (fixed 2.32.4) The third, GHSA-gc5v-m9x4-r6x2 (insecure temp file reuse in extract_zipped_paths), needs 2.33.0 and therefore 3.10. It stays open and is not reachable in JAWA's deployment shape: JAWA never calls that function, and its only caller inside requests is adapters.py passing DEFAULT_CA_BUNDLE_PATH, which returns immediately when the path exists. installer.sh builds a venv and pip-installs certifi as a real file on disk, so the zipped-egg branch that the advisory concerns is never entered. Revisit if the Python floor moves to 3.10, which would let the pin go to 2.33 and close it properly. Verified: 415 passed with requests 2.32.5 and Werkzeug 3.1.8.
The stated requirements said Ubuntu 20.04+ / RHEL 8.x+ and Python 3.8+. None of that has been true for a while, and the CI matrix disagreed with it too -- CI's oldest leg is 3.9, so the documented 3.8 floor was never tested. The installer builds the venv from the distribution's *default* python3 (apt-get install python3-*, yum install -y python3, then python3 -m venv), so the OS version decides the Python version. That gives: Ubuntu 20.04 3.8 fails: Werkzeug 3.1 requires >=3.9 Ubuntu 22.04+ 3.10+ ok RHEL/Rocky 8 3.6 fails, and has since JAWA moved to Flask 3 RHEL/Rocky 9 3.9 ok RHEL/Rocky 8 is the notable one: it stopped being able to run JAWA when Flask 3 arrived, independent of this release, and the requirements never caught up. Werkzeug 3.1 in this release is what rules out Ubuntu 20.04. Requirements now read Ubuntu 22.04+ or RHEL/Rocky 9.x+ and Python 3.9+, with a note explaining that the OS choice is what sets the Python version -- the failure mode otherwise is an opaque pip resolution error partway through an install as root. Also added to the release notes' upgrade section, since an operator on 20.04 or RHEL 8 needs to know before running the installer rather than after. Deliberately not raising the floor to 3.10 to close the remaining requests advisory: that would drop RHEL/Rocky 9 as well, and choosing who can run JAWA is a product decision, not a consequence of patching a dependency.
Reported by the maintainer: picking a credential set on the template enable form still prompted for the OAuth client ID. Two things were wrong. The form rendered every config_param unconditionally, including the three credential keys, while the dropdown hint promised it would "auto-fill server URL, client ID, and client secret" -- and nothing populated them. The template's own comment said as much. They were merely not marked required, so submit worked and the server used the saved set anyway. The worse half: substitute_params prefers the credential set over the form unconditionally, so a value typed into one of those visible fields was silently discarded. Nothing told the admin, and the only way to find out was to read the generated script. Fix hides exactly the fields the chosen set fills, and disables them so the browser omits them from the submission -- a stale typed value cannot reach the server to be dropped. Per-set, not blanket: only "name" is required when saving a credential set, so a set may carry any subset of the three. A field the set cannot supply stays visible and becomes required, which converts what used to be a server-side missing-value error at enable time into ordinary form validation. Deliberately NOT auto-filling the inputs, which is the obvious reading of the old hint. That would render the OAuth client secret into the DOM and into view-source. The page carries key NAMES only, asserted by a test that fails if anyone adds the values back -- verified by mutating the template to emit them. The hint now describes what actually happens, including that the saved set wins over anything typed. Also moves the param label off an inline style into a class, since new CSS belongs in main.css rather than the markup. 419 passed, ruff clean.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged the assertion I added in 87b3e70 as py/incomplete-url-substring-sanitization (high) -- the one new alert on the release PR, and self-inflicted. assert "https://typed.example.com" not in out It is a test assertion, not sanitization, so the finding is wrong about intent. But the pattern it matches is a real bug class: checking a URL by substring containment. Leaving it would mean asking the maintainer to dismiss an alert on a public repo for something introduced during release prep, which is a worse trade than rewriting two lines. Now compares the whole generated output line by line. That is a stronger assertion than containment -- it pins the exact emitted values including repr() quoting, and would catch extra or reordered output that "in" would not -- and it carries no URL substring check for the analyser to read as a security control. Behaviour under test is unchanged: the saved credential set wins for the key it supplies, and the form fills the key a partial set cannot.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JAWA v3.2.0
Release PR:
develop→main. 90 commits, 120 files. This is the single review gate forthe whole v3.2 line — the work was deliberately staged on
developfor one pass rather thanten separate reviews.
Please merge with a merge commit, not squash. The individual commits carry the reasoning for
each fix and are worth keeping on
main.How to review this
The merge commits are the entry points. Each is a self-contained piece of work:
12372944119061installer.shhardening + the CI guard that rejects invisible-unicode paste corruption in it2aff5dc716cac4869ea68data/cron.json154ea47c194a25Plus four commits made directly during release prep:
1781db4— the one real CodeQLpy/reflective-xsssurvivor (see Security below)f0ffe5d— v3.2.0 release notes rewritten to match what actually ships52c4e3d— login regression fix, reported against a live instance during prep8af3fb2,56615c7— dependency advisories cleared5aedf02— stated platform requirements corrected (see below)87b3e70,9c3a68f— credential-set fields on the template enable form (see below)Highest-value things to look at
52c4e3d— the login fix._verify_jamf_accessrequired a top-levelactivation_codekey from
/JSSResource/activationcode. Live Jamf Pro returnslicense_information, so theguard locked operators out of their own console and told them their URL was wrong. The
fixture mocked the invented shape, so the whole login suite validated the guard against a
contract that did not exist. Worth a look at the corrected mock as much as the fix.
154ea47— templates. This is the feature most users will touch. Two bundled templatespreviously failed immediately when triggered; config values containing
&, quotes or anglebrackets were corrupted on the way into the generated script.
1781db4— the receiver 401. Small diff, real vulnerability.Security
maincurrently shows 25 open CodeQL alerts. Disposition after this merge:views/cron_view.py,custom_webhook.py,jamf_webhook.pyand
okta_webhook.py, all of which this release deletes — the unified automation dispatcherreplaced them. These should close on their own once
mainhas this code; worth confirming.1781db4).webhook/jawa_receiver.pyinterpolated the caller-supplied hookname into a bare-string 401 body. Flask serves a bare string as
text/html, soPOST /hooks/<markup>with bad auth reflected unescaped markup onto JAWA's own origin. Thetwo sibling returns in the same function were safe only incidentally — they return dicts,
which are serialized as JSON. Now carries no request input at all, with a regression test.
views/home_view.pypy/full-ssrf(critical). The URL is theadministrator's own Jamf Pro server, entered at login. Connecting to the operator-supplied
server is what JAWA is for. To be annotated as accepted risk rather than dismissed silently.
resource_view.pypath-injection ×3 — guarded bysecure_filenameplus a containment checkthat the resolved directory equals the resources directory. This PR strengthens that guard.
resource_view.pyreflective-xss ×2 — the flagged returns are dicts rendered through Jinjawith autoescaping on.
jawa_receiver.pyreflective-xss ×2 — dict returns, serialized as JSON, not HTML-typed.develophas never had an independent CodeQL baseline, which is why the diff gate readspre-existing debt as new. This merge establishes the baseline so future PRs diff meaningfully.
Dependencies (
8af3fb2,56615c7): five open advisories, all medium, where thecompatible-release pins were blocking the fixes rather than lagging them —
requests~=2.31.0caps below 2.32/2.33 and
Werkzeug~=3.0.1caps below 3.1.4/3.1.5. Sinceinstaller.shinstallsfrom
requirements.txt, shipping unchanged would have put the vulnerable versions on every newinstall. Now
requests~=2.32.4andWerkzeug~=3.1.5.Four of five cleared.
requestsis held at 2.32.x deliberately: 2.33.x declaresRequires-Python >=3.10and the 3.9 leg of the CI matrix could not resolve it. Dropping 3.9 is adecision about who can run JAWA, not a side effect of clearing an advisory, so it stays. The two
advisories that matter for how JAWA uses
requestsare closed (theverify=Falsesessioncarry-over, and the
.netrccredential leak). The remaining one —GHSA-gc5v-m9x4-r6x2,insecure temp-file reuse in
extract_zipped_paths()— needs 2.33 and is not reachable here:JAWA never calls it, its only caller inside
requestsisadapters.pypassingDEFAULT_CA_BUNDLE_PATH, and that returns immediately when the path exists.installer.shpip-installs certifi as a real file into a venv, so the zipped-egg branch the advisory concerns is
never entered. Worth revisiting if the Python floor ever moves to 3.10. Werkzeug 3.1 emits absolute
Locationheaders for its own canonicalization redirects,which broke a test asserting
not startswith("http")as a proxy for "off-site"; the assertionnow compares parsed host against request host, which is both correct and stronger. The
open-redirect protection itself never regressed.
87b3e70— credential-set fields (maintainer-reported during review)Picking a credential set on the template enable form still prompted for the OAuth client ID. Two
defects:
promised it would "auto-fill server URL, client ID, and client secret". Nothing populated them —
the template's own comment conceded as much. They were just left un-
required.substitute_paramsprefers the credential set over the form unconditionally, so a valuetyped into one of those visible fields was silently discarded. The only way to notice was to
read the generated script.
Now hides exactly the fields the chosen set fills and
disableds them, so a stale typed valuecannot reach the server to be dropped. Per-set rather than blanket, because only
nameis requiredwhen saving a credential set — a key the set cannot supply stays visible and becomes
required,turning a server-side missing-value error at enable time into ordinary form validation.
Deliberately not auto-filling the inputs, which is the obvious reading of the old hint: that
puts the OAuth client secret in the DOM and in view-source. The page carries key names only, and
a test fails if the values come back (mutation-verified). Degrades to previous behaviour without
JS, since the server applies the same precedence either way.
Platform requirements changed — worth a reviewer's eye
5aedf02raises the stated minimum to Ubuntu 22.04+ / RHEL-Rocky 9.x+ and Python 3.9+,from Ubuntu 20.04+ / RHEL 8.x+ / Python 3.8+. This is a documentation correction, not a new
restriction invented here — but it is user-visible, so it is also in the release notes' upgrade
section.
The installer builds the venv from the distribution's default
python3(
apt-get install python3-*/yum install -y python3, thenpython3 -m venv), so the OS versionis what determines the Python version:
python3RHEL/Rocky 8 is the one to note: it stopped being able to run JAWA when Flask 3 landed, entirely
independent of this release, and the stated requirements never caught up. Werkzeug 3.1 here is what
newly rules out Ubuntu 20.04. Without the doc fix the failure mode is an opaque pip resolution
error partway through an install running as root.
The floor was deliberately not raised to 3.10 to close the last
requestsadvisory — thatwould drop RHEL/Rocky 9 too, and deciding who can run JAWA is a product call rather than a
side effect of patching a dependency.
Deliberately not in this release
own release rather than riding an 86-commit merge.
data/. The installer preserves theoperator's
data/across an upgrade, which protects their automations but also means thebundled template scripts and the webhook event catalog are not upgradable in place from 3.2
onward. Called out in the release notes' upgrade section. Inert for pre-3.2 instances, which
have neither file set — so it will not surface in a 3.2 upgrade test and needs a deliberate
answer in the update-mechanism work rather than being discovered later.
install; see the test plan.
Test plan
Mechanical (CI):
ruffcleanpytestgreen on 3.9 and 3.14installer.shbash syntax + non-ASCII guardLocally: 419 passed / 145 skipped, ruff clean, verified under both Werkzeug 3.0.6 (old pins) and
3.1.8 (new pins).
Maintainer gate before tagging — needs a clean VM and a dev Jamf instance:
installer.shon a clean VM. This is also the only real check of the nginxupload-cap change.
52c4e3dregression — a realinstance is the only thing that would have caught it.
requests(e.g.
teams-notificationordevice-naming, notevent-tracker-sqlite, which isstdlib-only) and read the script's log output, not just the HTTP response. The receiver
spawns scripts via the shebang, which resolves to system python rather than JAWA's venv,
so
requestsmay be missing. AModuleNotFoundErrorhere is J17 reproducing and blocksthe tag; a clean run means the distro carries
requestsand J17 drops to post-3.2. A 200proves nothing — the script's failure is only an exit code in a log.
Tag
v3.2.0only after that pass. The tag andmainmove together, sinceinstaller.shiscurled directly off
main.