Stable Release PR For 1.5.10.2482 - 2026-09-11 - #1761
Closed
fog-workflows[bot] wants to merge 227 commits into
Closed
Stable Release PR For 1.5.10.2482 - 2026-09-11#1761fog-workflows[bot] wants to merge 227 commits into
fog-workflows[bot] wants to merge 227 commits into
Conversation
merge stable - 1.5.10.2253 into dev
_createWebLeaf skipped reissue on a stamp that hashed ca.cnf alone -- the SAN set. Nothing in it identified the CA that signed the leaf, so pointing a server at a different Web CA with --web-ca-cert/-key/-root imported the new CA and then returned early without re-signing, because the names had not changed. The result was silent and complete: validateExternalCA reported success, the install finished clean, and the vhost went on serving the certificate signed by the CA that had just been replaced. Checking the served chain was the only way to find out, and it looked exactly like the flags had been ignored. Adds the signing CA certificate fingerprint to the stamp. Same CA and same names is still a no-op, so ordinary upgrades do not churn the leaf; either one changing now forces a reissue. Existing installs reissue once on the next run because the stamp format itself changed, which is also what heals a server already stuck in this state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Port of the same work on working-1.6, adapted where this line genuinely differs rather than copied. packages/pki/fog-mint-web-ca issues a name-constrained Web CA for another FOG server from this server's root, so every server's leaf chains to one root and a single certificate covers the fleet. It exists as a script rather than a documented openssl invocation because _defaultServerNames() puts fogserver and fog-server on EVERY leaf whatever the host is called, so a CA constrained to only the far server's hostname cannot sign the certificate it was minted for. The script probes for exactly that before emitting and refuses a CA that would reject its own server's certificate. Two adaptations, both because this line is not 1.6: Storage nodes here still generate their own self-signed CA -- issuance from the master is a 1.6 feature that is not present on this branch -- so the doc does NOT tell an admin their nodes are already handled. It says the opposite, and points them at the same procedure, because a five-node fleet on this line really does have six unrelated CAs and this is the only way to collapse them. The links differ too: EXTERNAL_CA_AND_LETSENCRYPT.md does not exist here, so the ACME discussion points at PKI_ZONES.md's own Let's Encrypt section, and the two PKI_ZONES.md anchors this branch lacks (storage nodes, bringing your own CA) are not linked. Every remaining link was checked to resolve on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…not have (#1023) Straight port of the transfer fix from working-1.6 (dbeb21b). Step 2 told you to `scp root@<hub>:/root/... /root/`, which cannot work at either end: sshd ships PermitRootLogin prohibit-password on most distributions, and an unprivileged local account cannot write to /root anyway. Both failures read `Permission denied`. It is now a push from the hub, staged through your own account, and extraction into /root/webca on the far side. Two flag claims were wrong for this line specifically, which matters more here than on 1.6 because this is the line a 1.5 user is told to move to: - "Both the 1.6 line and the 1.5 line have them; git pull" -- a released 1.5.x stable has no packages/pki at all, so `git pull` on a stable checkout never produces these flags. Replaced with a check the reader can run. - The zone table pointed at --secureboot-ca-cert, which does not exist on dev-branch. On this line you supply a Secure Boot key with --secure-boot-key/--secure-boot-cert. Also dropped the "--external-ca is not needed" line -- there is no such flag here to not need -- and fixed the options table describing Option B as copying a hub key out, which is the one thing the document later tells you never to do. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Quick Registration created the host but left the Inventory tab blank. FOS posts to service/inventory.php with the mac base64-encoded, but since 7251c3c that endpoint calls getHostItem(false), and getHostItem reads the mac with filter_input() on the raw request -- which stripAndDecode()'s rewrite of $_REQUEST never touches -- so the lookup ran against the still encoded blob, threw "Invalid MAC Address!" and never reached $Inventory->save(). Reproduced on the lab at 1.5.10.2259: a plain mac answers "Invalid Host", the same mac base64-encoded answers "Invalid MAC Address! MDI6MWY6M2U6NGQ6NWE6NmI=" -- the undecoded value echoed back. The reported one-line fix, getHostItem(false, true), only moves the failure: FOS sends the mac base64-encoded from fog.man.reg, fog.auto.reg and fog.download, but plain from bin/fog and fog.sysinfo, so forcing a decode breaks the standalone inventory task instead. Neither endpoint can know which encoding it is about to receive, so resolve it in the one place that reads the mac. stripAndDecodeMac() sniffs on shape, not on "do the decoded bytes happen to be valid UTF-8". That second test is unsafe for a mac specifically: a hex mac is built entirely out of base64 alphabet characters, so a plain mac decodes to accidentally-valid UTF-8 about once in every few hundred (measured 0.26% lowercase, 0.84% uppercase over 500k samples). Instead the plain value is kept when it is already a well formed mac list, and the decoded value is accepted only when it is one. MACAddress's validation pattern becomes a public const so the sniff and the class share one definition rather than duplicating the regex; it was being rebuilt with sprintf on every construction, and normalizeMAC() / isValid() now read the constant. Also hardened $vars['mac'] against an "Undefined array key" warning on the php://input fallback, in the same lines. Fixes #1036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fix #1036: inventory empty after Quick Registration (mac encoding in getHostItem)
Port of the working-1.6 fix. dev-branch carried a byte-identical
fog-version.sh and no pre-push at all, and was drifted by two builds when
this landed -- committed 1.5.10.2259 against a count of 2261, from merge
commits arriving without a re-stamp.
The pre-commit hook stamps FOG_VERSION from the branch name plus
"git rev-list master..HEAD --count", adding 1 because at pre-commit time
the new commit does not exist yet. That prediction is wrong in two
directions:
- A rebase, merge, cherry-pick, revert or --no-verify commit does not run
the hook at all, so the commit keeps whatever version it was stamped
with before.
- "git commit --amend" DOES run the hook, but an amend replaces HEAD
rather than extending it, so the +1 counts a commit that will never
exist and the version lands one too high. The documented remedy for the
first problem was therefore a cause of the second.
git gives a pre-commit hook no way to tell these apart: there is no
environment variable and no argument, and prepare-commit-msg -- which is
told (source=commit) -- runs after the tree is taken for an ordinary
commit, so it cannot stage the file. The first moment the answer is
knowable is once the operation has finished, so the check moves there.
fog-version.sh gains a "head" mode that answers "what should the commit
that ALREADY EXISTS say" with no +1, leaving modes 0 and 1 byte-identical
so pre-commit and fog-workflows' periodic sweep are unaffected. rc is
exempt: it increments off the committed suffix rather than a commit count,
so "correct" is by construction "one more than it is".
pre-push compares the two and refuses the push with the exact command to
fold the right value into HEAD. It fails open on anything it cannot
determine -- no master ref, no network, detached HEAD, a branch type with
no count-based version, or a push that does not include the checked-out
branch. Drift here reaches users through release notes on the Patches
channel, but a hook that guesses wrong and blocks a maintainer would still
cost more than the thing it prevents.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Installing with DHCP enabled aborts on Debian 13 and Ubuntu 26 at "Setting
up and starting DHCP Server (Kea)" with
Syntax check failed with: Unable to open file /etc/kea/kea-dhcp4.conf
for a file the installer has just written and can stat. It is not a syntax
error and the config is fine.
Those releases changed the kea package to ship /etc/kea as 0750 _kea:_kea
(it was 0755 root:root through Debian 12 and Ubuntu 24.04, which is why the
Kea support added for #730 tested clean). Their AppArmor profile grants
kea-dhcp4 "/etc/kea/** r" but deliberately withholds cap_dac_read_search
and cap_dac_override. configureKeaDHCP runs the syntax check as root, and
root is neither the directory's owner nor in its group -- so it needs a DAC
bypass just to traverse /etc/kea, AppArmor refuses, and Kea reports the
file as unopenable. The daemon itself was never affected: systemd runs it
as User=_kea, which owns the directory.
Run the syntax check as the config directory's owner instead. That needs no
DAC bypass at all, so it succeeds with the profile intact and unmodified.
Where /etc/kea is root-owned (RedHat, Arch, Alpine) the helper returns
nothing and the check runs as root exactly as before.
The reported workaround -- aa-complain, or deleting the profile -- is not
adopted here. Disabling a mandatory access control the distro shipped on
purpose is not something the installer should do to someone's machine, and
it is unnecessary once the check runs as the right user.
Also:
- chmod 0644 the generated config. A hardened root umask (027/077) would
otherwise leave it unreadable to _kea, which breaks the running daemon
and not merely the syntax check. 0644 is the mode the distro packages
ship it with and the generated config holds no credentials (memfile).
- On an "Unable to open file" failure, print what to actually look at
(dmesg for the AppArmor denial, ausearch for the SELinux one) instead of
the generic validation message, so the next person does not go hunting
for a JSON typo. Requested in the report.
Verified in debian:13, ubuntu:26.04, debian:12 and ubuntu:24.04 containers
with cap_dac_override and cap_dac_read_search dropped via capsh, which is
exactly the capability set AppArmor leaves kea-dhcp4 with: the old code
reproduces the reported message on the first two, the new code validates
successfully on all four, a root-owned /etc/kea still runs as root, a
genuinely malformed config is still rejected, and a config written under
umask 077 still validates.
Fixes #1039
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fix #1039: installer fails with DHCP enabled on Debian 13 / Ubuntu 26 (Kea + AppArmor)
…atches channel on dev-branch to match commit count since master
Route::edit() assigned every column back to its own current value when the request body did not mention it. That reads as a no-op and is not: User::set() hashes any non-override write to 'password', so a single PUT /fog/user/<id>/edit re-hashed the stored bcrypt hash. The real password no longer verifies against it and the account can never log in again -- while the request answers 200 and returns an object that looks correct. password_verify(plain, stored) = true password_verify(plain, rehashed) = false password_verify(storedHash, rehashed) = true Fixed by skipping fields the body did not send, which is what a PUT to /edit|update should have meant anyway. Safe because save() builds its statement from $this->data for every databaseField regardless of what was set(), so nothing else about the write changes. Ported from working-1.6 d8c5e52 (#1091), which also added the server-owned field guard that finding is really about. Only the lockout half is ported here: it is a plain bug fix, where the guard is new behaviour and belongs on the branch new capability lands on first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR
Stop a user PUT locking that account out permanently
Port of the working-1.6 fix. Verified present here before porting: 20 uses
in FOG-owned code, across four files.
route.class.php 12 defineRoutes()'s route table
bootmenu.class.php 3 one line in the menu item builder
location plugin 4 changeitems.hook.php's ipxe URLs
pending_mac_list 1
"${var}" was deprecated in PHP 8.2. The notice fires per EVALUATION rather
than per file, and the two heaviest sites are the API route table and the
boot menu builder, so this logged E_DEPRECATED on every API request and
every PXE boot. On an install with display_errors on it also reaches the
output stream, which is how a deprecation notice turns a JSON response into
an unparseable one.
Rewritten as "{$var}", which evaluates identically -- worth stating because
12 of the 20 are API route patterns.
DONE WITH THE TOKENIZER, NOT A REGEX. This tree holds a lot of iPXE script
text in SINGLE-quoted PHP strings ('chain -ar ${boot-url}/...', 'set arch
${buildarch}'), which is iPXE's own syntax, does not interpolate, and must
reach the client byte for byte. A text search for '${' corrupts every one
of them. The tokenizer only reports T_DOLLAR_OPEN_CURLY_BRACES inside a
genuinely interpolating string. Verified after: zero lines mentioning
boot-url changed.
THE VENDORED LIBRARIES ARE LEFT ALONE. This branch's older copy of
lib/db/mysqldump.class.php carries 30 more of these. It is
ifsnop/mysqldump-php and a swap candidate for its Packagist release, so
hand-editing it makes that swap harder and would be reverted by it anyway.
working-1.6 already carries a newer copy with none, which is the shape the
real fix takes. tests/php-deprecations.test.php exempts the three vendored
files by name and says so.
That test is new here and runs standalone -- this branch has no
tests/run-all.sh. Negative-tested by reintroducing one:
FAIL: 1 use(s) of "${var}" interpolation, deprecated in PHP 8.2:
packages/web/lib/reports/pending_mac_list.report.php:231
NO OPENAPI CHANGE, and route.class.php is touched: no route was added,
removed, renamed or re-shaped. The edits change how a pattern string is
written in source, not what it evaluates to.
NOT DONE HERE, deliberately: this branch spells FilesystemIterator as
FileSystemIterator in five places. On working-1.6 that was a single outlier
against eleven correct spellings and worth resolving; here it is uniform,
so there is no inconsistency to fix and changing it would be churn on the
patches line.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR
Drop deprecated "${var}" interpolation
The approve/delete confirmation built both of its sentences by
interpolating an action word into a _() call:
echo _("MACs $msg successfully");
echo _("The follow MACs have been {$msg}.");
Two problems, one of them invisible.
The visible one is a typo: "The follow MACs" should read "The
following MACs".
The invisible one is that neither line could ever be translated.
xgettext extracts the *literal source string*, so the msgid that
reaches the catalog is "MACs $msg successfully" -- but at runtime PHP
interpolates first and gettext is asked to look up "MACs approved
successfully", which is not a key in any catalog. The lookup misses
and returns its argument unchanged, so both lines rendered in English
on every install regardless of language. It also gave translators no
way to inflect the sentence around the action word, which several
languages need.
Build the whole sentence in each branch instead. That is four
complete msgids xgettext can actually extract, and the typo goes with
it. $title/$body are initialized so a POST carrying neither button
does not warn on an undefined variable.
Only dev-branch is affected -- working-1.6's copy of this report was
rewritten and no longer contains either string.
xgettext extracts the LITERAL SOURCE TEXT of a _() argument, but PHP
evaluates that argument before _() is ever called. Where the two
differ, the msgid sitting in the catalog and the string looked up at
runtime are different strings and the lookup can never hit:
echo _("Add selected $getType");
msgid in messages.pot: "Add selected $getType"
looked up at runtime: "Add selected Image"
gettext misses, returns its argument unchanged, and the string renders
in English on every install whatever language is selected. Nothing
errors and nothing logs. The only symptom is a string that is never
translated, which is invisible to anyone testing in English -- which is
why seventeen of these had accumulated.
Fixed the seventeen sites the same way throughout: the literal goes
inside _(), the variable is substituted outside it, which is already
the house style ($foglang['ListAll'] is literally _('List All %s')).
That hands xgettext a complete format string and hands the translator
a placeholder they can move, which matters -- several shipped
languages cannot keep English's word order.
Two sites needed something other than sprintf:
buildMenu() wrapped _() around a sprintf that produced a class name
with an 's' on it. The outer call was a guaranteed miss returning its
own argument, so it is simply removed -- a no-op at runtime.
fogservice.class.php's _("{$itemType}s") becomes _($itemType) . 's',
matching the _($itemType) already sitting three lines away in the same
expression. Same rendered output, and it can now resolve.
The new gate draws its line at "does this msgid mix literal text with
a variable". It deliberately still ALLOWS a bare runtime lookup such
as _($node): that resolves correctly whenever the value is itself a
msgid extracted from some other literal site, which is how FOG
translates node and class names. Fragile, since xgettext cannot verify
it, but not broken by construction the way the mixed form is -- and
the distinction needs no allowlist, because an argument either carries
literal text or it does not.
Not fixed here: fogservice.class.php builds its log lines by
concatenating separately-translated fragments ('Not syncing' + type +
'between' + ...). That is a real i18n problem, but rewriting those
into whole sentences changes daemon log output people grep, so it
wants its own change.
Ports the same sweep landed on working-1.6.
Port of the same two fixes landed on working-1.6. 1. service/ipxe/advanced.php never authenticated anything. FOG_ADVANCED_MENU_LOGIN is documented as enforcing a login for the advanced menu. The file serving that menu never read the setting, and its final printf of FOG_PXE_ADVANCED sat outside every conditional, so the menu went to any caller regardless of setting or credential. The login was decorative three ways over: the setting was never consulted, attemptLogin() returns a User object on both paths so `if ($tmp)` could never be false, and the success branch built $Send but never parsed it before chaining back with no credential at all. Restructured so the credential and the decision to emit the menu happen in the same request -- iPXE carries no cookie, so there is no session in which an earlier login could be remembered. The default is preserved: FOG_ADVANCED_MENU_LOGIN defaults to 0, so installs that never enabled it see no change. 2. Authenticating minted a session for callers that cannot hold one. validatePw() stamps $_SESSION['FOG_USER'] and runs _isLoggedIn(); three of its callers here are iPXE endpoints, which have nowhere to keep it. Added authenticate() plus FOGBase::authenticateOnly() and repointed the two bootmenu callers and advanced.php at them. DELIBERATELY NARROWER THAN working-1.6. There, validatePw() was split into authenticate() + establishSession(). Here it is not, because this branch's validatePw() still contains the LDAP-era branch that signs a user in when passwordValidate() FAILED, provided a hook has populated self::$FOGUser. Extracting that faithfully would mean blessing it, and tightening it is the separate open LDAP/RBAC work -- not something to do inside a fix about session creation. So authenticate() here mirrors validatePw()'s acceptance exactly, including that branch, and changes only whether a session is created. Who gets in is identical before and after; LDAP logins at the PXE menu keep working. tests/ipxe-auth-no-session.test.php gates it, with the establishSession() assertion made conditional so it holds on both branches and tightens automatically if this one is ever split too.
Port of #1113 from working-1.6, re-verified against this tree rather than assumed -- the check that makes the change safe is a property of 1.5's own files, not of 1.6's. Initiator::__construct() called session_start() unconditionally, and 59 entry points reach it through commons/base.inc.php: 40 files under service/, 13 under status/, the API and the iPXE endpoints. None of those can carry a cookie back, so every one of those requests allocated a session record that was written once, never read again, and left for gc. A PXE boot, or a fleet of fog-clients polling on a timer, is a steady stream of them. session_start() now runs only when the caller presents a session cookie or the entry point declares FOG_WANTS_SESSION. management/index.php is the sole declarer: the login form's CSRF token needs somewhere to live before anyone has logged in, so a cookie-less visitor to the web UI still gets a session. Re-ran the load-bearing check on this branch: a tokenizer sweep for $_SESSION across packages/web finds not one write under service/, status/, lib/reg-task, lib/client or lib/service. Everything that does touch session state is either a browser page or already guards on session_status() -- setMessage(), getMessages(), resetRequest(), Initiator::language(), LoadGlobals's currentUser lookup and User::_isLoggedIn()'s regenerate branch all check first. The iPXE endpoints stopped entering validatePw() in #1112, so nothing browser-less writes FOG_USER any more either. tests/no-session-for-browserless.test.php pins all three properties: the guard is present (comments stripped first, so the block explaining the guard cannot satisfy the search for it), the web UI is the only declarer, and browser-less directories stay free of $_SESSION. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR
Stop handing a session to entry points that cannot carry a cookie
Port of the working-1.6 sweep, and the drift is worse here: 427 files opened with "PHP version 5" while Initiator::_verCheck() has thrown below 7.4 for years and CONTRIBUTING.md says "Target PHP 7.4+". Only the file headers were still describing the 1.x era. 430 lines across 423 files, including three that said "PHP version 7" without the minor, normalized to the "PHP version 7.4+" form. Comment text only: token streams with comments and whitespace stripped are byte-identical to HEAD in all 423 files. tests/php-version-docblocks.test.php reads the floor out of _verCheck() rather than hardcoding it, so a future bump fails until the docblocks follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR
Say 7.4 in the docblocks, which is what FOG has enforced for years
…atches channel on dev-branch to match commit count since master
… hosts (#1133) Port of working-1.6 #1132. Same two problems, same fix, adapted to this branch's shape -- and one extra bug that only exists here. FOGURLRequests makes every outbound HTTP request the web tier makes: the GitHub kernel and init listings, the kernel download itself, the fogproject.org version check, and the storage-node status endpoints. It treated all of them identically, in two ways written for the last of those and then silently applied to the rest. CURLOPT_SSL_VERIFYPEER and VERIFYHOST were false for everything. That is genuinely necessary for a storage node -- addressed by bare IP, presenting whatever certificate the installer generated. It is not necessary for api.github.com or fogproject.org, which present ordinary publicly-signed certificates, and there it bought nothing while costing the ability to notice a substituted response. The kernel download is the sharpest case: the file lands on disk and is then booted by every machine that images. And the signed-in administrator's PHP session id was attached to every request, along with the CSRF token. Both are there so a node's status endpoint can authorise the call; sending them to api.github.com on every kernel listing is a credential handed to a third party for no purpose. Both are now decided by the URL's host rather than by the caller, so a new caller cannot acquire either by not thinking about it -- which is how every request this class made came to be unverified in the first place. The exemption is one named constant, NODE_TLS_OPTIONS, applied only when isFogHost() says the host is one this install owns: a storage node, this server, or loopback. A caller that names CURLOPT_SSL_VERIFYPEER itself still wins. The extra bug, and the reason this port is not a straight copy: the node list was read as getSubObjectIDs('StorageNode', array('isEnabled' => 1)) with no field argument, and getSubObjectIDs() defaults $getField to 'id'. So the list was storage node IDs, not addresses, and the pattern built from it was '#1|2|5#' matched against the whole URL. The only consequence was a proxy applied essentially never -- any URL containing one of those digits looked like a node -- which is why nobody noticed in ten years. It is not survivable once the same answer decides whether a certificate is checked, so the read now asks for 'ip' and the gate pins that it does. The comparison is a whole-host one rather than a pattern for the same reason. Even with real addresses, '#' . implode('|', $IPs) . '#i' is unanchored with the dots unescaped, so a node at 10.0.0.5 also matched https://example.com/?ref=10.0.0.5. Behaviour change worth stating: an install whose outbound traffic is TLS-intercepted by a corporate proxy will now fail to fetch the kernel list and the version check until that proxy's CA is trusted by the OS. That is the correct outcome and the normal fix, but it is a visible change on a patch line. tests/url-requests-tls.test.php runs the host matcher for real (a pure static, so no database) over the substring cases the old regex got wrong, and pins the wiring by shape. Verified failing by restoring the id-not-ip read and by un-gating the session cookie (2 failures).
This branch carries seven tests and nothing runs them. FOGProject/fog-workflows holds all of this repository's CI and, until working-1.6 got a stub, every workflow there was scheduled or dispatch-only -- so these gates only ever ran when somebody remembered to run them by hand. Two files, because a stub alone would fail here: working-1.6 has tests/run-all.sh and this branch does not, so the runner is ported alongside it. Kept as close to identical as the branches allow, with three claims corrected rather than copied -- there is no docs/adr/ here to cite, no secureboot-authvars.test.sh to name as the skip example, and this branch's pre-commit has no blocking gate, so "adding a second one" was wrong. A pull_request trigger can only fire from the repository the PR is opened against, which is why the stub lives here rather than in fog-workflows, and why it is needed per BASE branch: GitHub reads workflows from the merge of head into base, so one copy here covers every PR opened against dev-branch and a contributor's own branch needs nothing. Not a push trigger. The runaway that put ~30 commits on this branch in 20 minutes happened because the triggered workflow pushed a commit back; the reusable workflow writes nothing and holds no token beyond the caller's read-only GITHUB_TOKEN. Verified before wiring it up rather than after: all seven pass on PHP 8.3 and, in a php:7.4-cli container, on the 7.4 floor the matrix also builds. So CI is green from the first pull request rather than starting red. Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Port of working-1.6 fffb637. The bug is present here unchanged: the same `-d $backupPath/fog_web_<ver>.BACKUP` gate, and the same cause behind it -- configureHttpd() removes ${docroot}fog when it is a symlink and then tests `-d $webdirdest`, which IS ${docroot}fog/, to decide whether to copy the old tree aside. On any install whose web root is a symlink that directory is never created, so the pre-upgrade dump was silently skipped on every run and the step printed "Skipped", which reads as "there was no prior install". Ask the database instead. SHOW TABLES against $mysqldbname is the actual precondition for a meaningful dump, and it has nothing to do with the web tree. configureMySql runs before backupDB here too (installfog.sh 763 and 791), so $sqloptionsuser and $snmysqlpass are settled; a fresh install has no tables and still skips. Only the gate and the filename are ported. This branch fetches with wget and --post-data rather than curl piped through jq, and that is left exactly as it was -- the change is to whether the fetch is attempted, not to how. Verified by execution against the 1.5 tree on the lab box, all three branches: a real database with the endpoint reachable dumps 561,547 bytes across 65 CREATE TABLE statements and reports Done, a non-existent database reports Skipped, and a real database with the endpoint unreachable reports Failed. The web-tree half of the same bug -- the lost fog_web_<ver>.BACKUP, and with it the management/other/ carry-forward that reads it -- is GH-1146, and it is present on this branch too. It stays separate because fixing it means changing which path the installer removes when the web root is a symlink. Also ported: the dump filename used date +%I, the twelve-hour clock with no AM/PM marker, so an update at 05:57 and one at 17:57 on the same day produced the same name and the second overwrote the first. %H. Refs #1146 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
configureHttpd() removes ${docroot}fog when it is a symlink, and then four
lines later tests `-d $webdirdest` to decide whether to copy the old tree
aside. $webdirdest IS ${docroot}fog/ -- the path just unlinked -- so on any
install whose web root is a symlink the test failed, no fog_web_<ver>.BACKUP
was written, and the tree behind the link went unsaved.
The louder consequence is further down the same function. The loop that
carries an admin's own files in management/other/ across an update reads that
backup directory, so it found nothing, every run, and said nothing about it.
The only trace was a find(1) complaint buried in the error log:
find: '/home//fog_web_1.6.0-beta.3562.BACKUP/management/other/': No such
file or directory
Resolve the link before dropping it and keep the target in $priorwebdir, then
fall back to it when $webdirdest is no longer a directory.
Copy only, no removal, on that fallback path. The existing branch deletes
$webdirdest because the new tree is about to be written over that exact path;
$priorwebdir is somewhere else the admin chose and was already being left
behind before this change. Backing it up is the gain -- deleting it would be
new destruction nobody asked for, and the difference matters for a web root
symlinked onto a separate volume.
A link resolving to the document root itself, or to one of its parents, is
refused outright. That is not a FOG tree to copy aside, it is somebody's
whole web server, and GH-953 is the standing reminder of what taking such a
path at face value costs here.
Verified by extracting the edited region verbatim out of functions.sh and
running it against six fixture layouts: a symlinked web root now produces the
backup with management/other/ intact, the accesscontrol plugin still stripped,
and the target left in place; a real directory behaves exactly as before,
backed up and removed; a link to the document root, a link to its parent, a
broken link and no link at all each produce no backup, delete nothing, and
leave a clean error log.
Port of working-1.6 aa231c0. The block is identical on this branch --
same unlink, same `-d $webdirdest` test on the same path, same
accesscontrol strip -- so the change is byte-for-byte the same, and the
same six fixture layouts were run against the region extracted from this
copy of the file.
Closes #1146
Port of the working-1.6 stub, byte-identical on purpose. FOG_VERSION is derived from the commit count since master, and the pre-commit hook that keeps it right is client-side -- so a PR merged through GitHub's web UI never runs it, and until now the only thing that noticed was fog-workflows' 10:10 UTC sweep. This reacts to the merge instead. The file is the same on both branches rather than scoped to each one, for the same reason tests.yml is: fixing it should not mean editing it on three branches. GitHub reads a pull_request_target workflow from the PR's base branch, so this copy only ever acts on merges into dev-branch; the allowlist is what scopes it. pull_request_target, not push, and that is the entire safety argument. The sweep pushes its fixup commit straight to the branch, and a direct push is not a PR merge, so it cannot re-fire this stub. The 2026-07-28 runaway that put ~30 commits on this branch in about 20 minutes was a push-triggered stub doing exactly that. Two consequences specific to dev-branch. stable's version is computed as the commit count of master..dev-branch, so the extra sync commits here make that number advance faster -- larger, but never wrong, since that count is what the version is defined as. And stable-releases.yml's final `gh pr merge stable --merge` has dev-branch as its base, so the release flow will trip this stub and re-sync dev-branch once the release lands. That is correct (the count really did change) and cannot loop, because the sync's own push is not a PR merge. Requires FOGProject/fog-workflows to move its version-badge write onto an App token first: that step writes to fog-workflows using github.token, which under workflow_call is the caller's token and cannot write to another repository. Without that change every merge here goes red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PqufBbuckux8kitJeW3uAK
Sync the version on merge, not just on the daily sweep (dev-branch)
Port of #1159 to the 1.5 line. Verified present here first: checkInternetConnection() was byte-identical to the version that was fixed there, and every fetch it guards had the same unbounded shape. "Testing internet connection" could sit silent for minutes: - It opened by running `$packageinstaller curl`, so a connectivity check's first act was a package transaction needing the connectivity it was about to test -- metadata refresh against every mirror, unbounded on Debian/Ubuntu whenever unattended-upgrades holds the dpkg lock (apt has no lock timeout), and on Arch a full system upgrade, because $packageinstaller there is `pacman -Syu`. curl is in every distro's package list and installPackages runs before anything that uses it, so the pre-install was redundant as well as expensive. - No timeouts on anything: libcurl defaults to a 300 second connect timeout and no total timeout, so each unreachable host cost five minutes, per address family. getent hosts has none to give it either. - Nothing read the result. dns_ok/http_ok/https_ok were set and never looked at, both failure paths returned rather than exited, and the caller ignored the status. It probed httpbin.org, neverssl.com, github.com and fogproject.org. It now probes the host behind $ipxegit/$ipxeurl -- the iPXE sources and the iPXE and Secure Boot release assets -- so an override to an internal mirror is tested instead of github.com rather than as well as it. One HTTPS request settles DNS, TCP and TLS together and curl's exit status names which failed. No -k: a proxy presenting its own CA passes an unverified probe and then fails the git clone that follows. The result is now $internet_ok, and the fetches read it: - downloadfiles() is the worst of them, and it runs on every install and upgrade: 8 URLs x 10 rounds x 2 timeout-less curls = 160 unbounded connects, silent under one "Downloading kernel, init and fog-client binaries" line. - fetchipxeasset() did 10 rounds of two timeout-less curls. - prepareiPXEsource() skips its git fetch when the host is unreachable; git has no connect timeout of its own, and that fetch is only ever an update to a checkout that already works. - The udpcast config.guess/config.sub wgets had no connect timeout and wget defaults to --tries=20. --max-time is deliberately not used on the artifact downloads: those are multi-megabyte kernels and tarballs, and a slow but working link must be allowed to finish. --speed-time/--speed-limit catch a transfer that opens and then stalls. Differences from the 1.6 change, all because this branch predates the splits: no $pluginsgit/$pluginsurl to probe and no bin/fetch-plugins.sh (the plugin repo split is 1.6-only), and no _ensureEfitools(). This branch also uses wget where 1.6 uses curl for its $ipaddress-local calls, so the test's wget rule excludes those by name. Offline installs are unaffected: failure stays non-fatal, the checksum of a pre-placed tarball is still tested before any download is attempted, and a pre-placed source tree is still used as-is. tests/network-fetch-bounded.test.sh ported with it; run-all.sh already globs *.test.sh. All 13 assertions mutation-verified against this branch's source. Full suite: 8 passed, 0 failed. The RH DNS advice pointed at /etc/sysconfig/network-scripts/ifcfg-*, which has not been where DNS lives since NetworkManager keyfiles; it now names nmcli. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bound every installer network fetch (port of #1159)
…er.sh There was no working supported path from a 1.5 server to 1.6. utils/FOGUpdater/fogupdater.sh looked like one: it had a Beta arm mapping to working-1.6. That arm has never worked. It resolves a branch's version from packages/web/lib/fog/system.class.php, a path 1.6 does not have -- it moved to packages/web/src/Base/System.php -- so the fetch 404s, the version comes back empty, and every crossing dies reporting a version lookup failure. Its verifyPayload() reads the same missing path, so fixing one half only moves the failure a few lines down. GH-1587. It also ended in `./installfog.sh -y`. Unattended is defensible for a 1.5.x point update, where .fogsettings already holds every answer. It is the wrong default for a crossing that meets settings that file has never held and takes a default for each of them without saying so. So it is retired rather than repaired, and bin/updatefog.sh replaces it -- git-based, beside the installer, matching where the 1.6 line already keeps its updater. That also leaves the server a git checkout, so 1.6's own updatefog.sh works immediately afterwards; a tarball extraction does not. THE HAZARD THE SCRIPT IS BUILT AROUND It replaces itself. Checking out 1.6 rewrites bin/updatefog.sh while bash is still reading it, and bash reads a script incrementally, seeking by byte offset -- so a file that changes length mid-run makes it resume in the middle of a different line. Silent, arbitrary, and it happens AFTER the checkout has succeeded, which is the worst possible moment: the code has moved and the process driving the upgrade is executing fragments. So the first thing it does is copy itself out of the checkout and re-exec. The test proves this by emptying the original mid-run and asserting the process finished anyway, rather than by grepping for the function -- this is exactly the kind of guard that gets simplified out by someone who has not hit it. Everything else is a refusal: a tarball install (no .git) is sent to the bootstrap one-liner, which clones and upgrades in place; an unknown channel is separated from a known channel with nothing published; no terminal and no --yes exits rather than going unattended; an unanswered confirmation cancels rather than being read as consent. The crossing is announced as a major upgrade and points at bin/revertfog.sh, which restores the pre-upgrade database dump -- the only supported way back. A failed install reverts nothing and names the commit to reset to instead. Standalone by design. 1.5 has none of the channel map, managed .fogsettings keys or helper functions the 1.6 updater shares with its installer, and back-porting them would mean surgery on lib/common/functions.sh on a line heading for end of life. It will diverge from the 1.6 implementation, and that is fine: this line is terminal. fogupdater.sh is kept as a message rather than deleted, so an existing cron entry gets an instruction instead of "No such file or directory", and exits non-zero so a scheduled run reports a failure rather than success forever. Its test is repurposed rather than removed: it now asserts the file fetches nothing and runs nothing, which is the GHSA-qp3r-8mwm-vg6h guarantee in its strongest form and a tripwire for anyone reintroducing a download here. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XtZHpyWZWDPSxePdTViGZy
…status-dev Let the pre-commit hook exit 0 when an optional tool is missing (GH-1581, dev-branch)
…atches channel on dev-branch to match commit count since master
ls-remote matches a pattern against the TAIL of each ref at slash boundaries, so the bare 'rc-*' pattern also matched refs/heads/feat/rc-update-channel -- confirmed against origin. On --channel rc, which is the channel this script recommends for the crossing, a 1.5 server would have been checked out onto a feature branch and told it was the current release candidate. The pattern is now refs/heads/rc-*, and the extracted name is re-checked with a sed that rejects a further slash. No assertion here could have caught it: every other one runs against a git stub whose ls-remote exits 0 saying nothing. rcBranch is now lifted out and run against a real bare repository carrying the decoy. Confirmed to go red against the old pattern. Co-Authored-By: Claude <noreply@anthropic.com>
Add bin/updatefog.sh for the 1.5 -> 1.6 crossing, and retire fogupdater.sh
…atches channel on dev-branch to match commit count since master
…anything Two defects found reviewing this after it merged. The self-copy read "$0", and the script cd's to its own bin/ forty lines earlier. Invoked by a relative path -- `bash bin/updatefog.sh` from the checkout root, or a cron entry with a relative path -- $0 is `bin/updatefog.sh`, which stops resolving the moment that cd happens. The copy failed and the script exited, and the copy is the one thing it cannot run without: it exists so a checkout cannot rewrite the file being read. readlink -f "$BASH_SOURCE" was already being computed to find bindir, so it is kept in $selfpath and the copy reads that. The /dev/tty test happened AFTER the checkout. A headless run without --yes -- cron, CI, a container with no tty -- moved the working copy to 1.6 and only then discovered it could not run the installer, leaving a 1.5 server with a 1.6 source tree. The message did disclose that and print the reset command, but the right answer is not to get there: bin/bootstrap.sh tests the terminal before it clones, and this now tests it before the confirmation prompt, which is before any git work. Nothing has moved when it refuses, and the message says so rather than explaining how to undo something that did not happen. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XtZHpyWZWDPSxePdTViGZy
…nd-tty Copy from the resolved path, and refuse a headless run before moving anything
…atches channel on dev-branch to match commit count since master
The same two shapes fixed on working-1.6, here. Both revert suggestions print `git checkout --detach <commit>` instead of `git reset --hard <commit>`. This script's whole purpose is the crossing, so the case is guaranteed: the checkout is on working-1.6 or an rc-* branch while $currentCommit belongs to stable, and resetting points the NEW branch's ref at an old commit, leaving it diverged long after the install is sorted out. Detaching moves only HEAD, which is all that is wanted, and the next run checks out a branch again anyway. That is the opposite of what a `reset --hard origin/<branch>` is for -- that one discards local mess so an update can proceed at all, and stays. Going back through history is a different job. And the confirmation stops treating EOF as a deliberate "no". `read` returns empty with nobody at the keyboard, the catch-all printed "Canceled." and exited 0, and the caller recorded a successful update that never happened. The terminal check added earlier catches the usual headless case; this covers a terminal that exists but answers nothing. A real no still exits 0; an empty answer and an unrecognized one do not. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XtZHpyWZWDPSxePdTViGZy
…-eof Detach rather than move a branch ref, and do not read EOF as a decision
…atches channel on dev-branch to match commit count since master
…tes a task Port of #1678 from working-1.6. Inside the method's imaging branch there were only multicast and deploy arms. A capture matched neither, fell out the bottom without inserting anything, and the caller reported success over an empty tasks table (#1677). On this branch the group task page never offers a capture, but POST /group/{id}/task accepts any task type and answered a capture with a 201 and no row. Capture shares the deploy insert: the two differ only by typeID, which is already a parameter. Capture is a one-host task type (ttIsAccess= 'host') and several hosts writing one image at once would corrupt it, so the arm refuses more than one host rather than create the race. tests/group-capture-tasking.test.php drives the real method against a fake database with the node's online probe stubbed, and reads the INSERT into tasks: one-host capture writes one capture row on the master node, two-host capture is refused with nothing written, two-host deploy still writes two rows. Six of its eleven checks fail on the code before this commit. Refs #1677 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0131h4axUaE3VJZphvKRdDby
…sking-dev Give Group::createImagePackage() a capture arm so a group capture writes a task
…atches channel on dev-branch to match commit count since master
… as BIOS Port of #1696 to dev-branch. The memtest menu entry chained memdisk with the 2013 Memtest86+ 5.01 ISO. memdisk is 16-bit real-mode code, so on a UEFI client the entry failed outright (#321). Memtest86+ 8.10 ships one hybrid file per architecture that is both a bzImage and a PE, so iPXE can `kernel` it on BIOS and `chain` it on UEFI with no ramdisk in between. - Add the upstream mt86plus_x86_64 and mt86plus_i586 binaries; a test pins their sha256 and both boot headers. - The boot menu picks the entry by ${platform} and refuses on ARM, where upstream publishes no build. - Schema 288 moves FOG_MEMTEST_KERNEL from the old default only. - The installer countersigns both files for Secure Boot alongside the FOS kernels. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0131h4axUaE3VJZphvKRdDby
Boot Memtest86+ 8.10 directly so the memory test runs on UEFI as well as BIOS
…atches channel on dev-branch to match commit count since master
Selecting Chinese on a Rocky Linux 9.8 server running PHP-FPM returned the translated interface as `???` -- already in the HTML, so not a browser, font or cache problem. The response declared charset=UTF-8 and the .mo catalog held valid UTF-8, but neither of those decides the bytes gettext hands back: glibc converts a translation from the catalog's charset to the codeset of the active LC_CTYPE, and substitutes a literal `?` for anything the target cannot represent. Initiator::language() set LC_MESSAGES only. A php-fpm pool commonly starts with LC_CTYPE=C, whose codeset is ASCII, so the correct catalog was selected and then flattened on the way out -- `Dashboard` came back as three bytes, 3f3f3f. bind_textdomain_codeset() fixes it at the domain rather than by setting LC_CTYPE, which would need a zh_CN.UTF-8 (or ja_JP.UTF-8, ...) locale generated on the host. Every language is affected, not just Chinese; the damage is simply less visible one accent at a time. The test observes gettext's own state, never the source text, in two halves. The first asserts the domain is left emitting UTF-8 for all nine languages -- on PHP 8 only, because the null query is an 8.0 capability and 7.4 would coerce it to an empty string and set rather than read. The second is the reported failure end to end: LC_CTYPE forced to C, the tracked .po compiled by msgfmt into the real tree and handed back afterwards, and the result checked for the reporter's 3f3f3f. It stands down out loud where zh_CN.UTF-8 is not generated, since gettext cannot select a catalog for a locale the host does not have. Both halves were made to fail with the fix removed. Reported-by: tangcenwu Closes #1720 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEqEBT67DfLDm6hm51BaUX
…tf8-1.5 Bind the gettext domain's output codeset to UTF-8 (1.5)
A re-run of installfog.sh on a Kea install wrote the Kea JSON to the distro's ISC config path. On Ubuntu that is /etc/dhcp3/dhcpd.conf. AppArmor denies kea-dhcp4 that path, so validation fails. The install then stops before TFTP, iPXE, FTP and NFS are set up. resolveDHCPEngine() swaps the ISC package for the Kea package in $packages, and the installer saves that list. It does not save $dhcpname or $dhcpconfig, so the next run re-seeds both to the ISC values. The guard looked only for the ISC package name in the saved list. It found none, and returned before it set $dhcpconfig. The first run always worked, which is why this was missed. The guard now accepts the Kea package too. With the saved engine "kea", the re-run sets the Kea package, service and config path again. An ISC install is unchanged: its list still names the ISC package, and its saved engine is "isc". tests/dhcp-engine-rerun.test.sh replays two runs through the real function for ubuntu, redhat and arch, with each distro's own defaults read from lib/<distro>/config.sh. It also pins which keys are saved, because the re-run it replays depends on that. Before this change it failed 6 assertions: the re-run config path and package name on each distro. After it, 31 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012JsEqysRdWb914YC94Shvo
Keep the Kea config path on an installer re-run (GH-1747)
On an interface with more than one IPv4 address, the installer wrote a DHCP subnet that was not a network. The report shows a saved mask of 255.255.0.0 on a /24, a Kea subnet of "10.0.45.2/24", a pool starting at .12, and "arithmetic syntax error ... /8" during interface selection. Each helper took "the interface" and read a different address from it: - input.sh took every inet address, link-local 169.254.x.x included. Listed first, a link-local address became the primary. - getCidr() printed the SECOND global address's prefix (head -n2 | tail -n1), through an unanchored grep, so eth1 also read eth10. A global 169.254.x.x/16 next to a /24 gave 255.255.0.0. - configureDHCP and writeKeaSample passed every global address, unquoted, to mask2network, so a second address became the mask. 10.0.45.2 with 10.0.45.14 gave the network 10.0.45.2. - interface2broadcast() took the first brd, whichever address it belonged to. Now everything derives from $ipaddress, the address FOG advertises as next-server: - input.sh takes global addresses only, and never 169.254.0.0/16. - getCidr IFACE [ADDR] prints the prefix of ADDR, or of the first global address. It reads only that interface. - interface2broadcast IFACE [ADDR] prints the brd of ADDR. - configureDHCP and writeKeaSample use $ipaddress for the network. Also: - cidr2mask with no prefix returns nothing instead of printing the /8 arithmetic error. - mask2cidr: "224)" had `let` and `nbits+=3` on separate lines, a string append, so a /27 mask came out as 243. Its error message went to stdout, where every caller takes the value as the prefix; it now goes to stderr. - The ifconfig fallback in input.sh is removed. It stored mask2cidr's prefix length in $submask, and ifconfig is not installed before the package step. An empty mask is derived again in configureDHCP. - With -y, an interface with no usable address now exits. It used to repeat the same answer forever, and the link-local filter adds a way to reach that path. tests/installer-subnet-detection.test.sh replays address layouts through the real functions and the real input.sh lines, with a fake `ip` built from real `ip -o` output. On the old code it fails 33 of 47 checks and reproduces the report: "10.0.45.2/24", pool "10.0.45.12 - 10.0.45.254", the saved 255.255.0.0, and the /8 error. It passes all 47 after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012JsEqysRdWb914YC94Shvo
Derive the DHCP subnet from the FOG address, not the interface (GH-1747)
fog-version.sh took the newest tag of any name as the base version. The
tag archive/feature-fog2-gui, pushed on 2026-09-06, is now the newest
tag, so the dev arm computes archive/feature-fog2-gui.2479. The slash
breaks the sed in apply-fog-version.sh, and fog-workflows' daily sweep
has failed on dev-branch every day since. The stable arm has the same
defect.
Both the tag lookup and git describe now match release tags only
('[0-9]*'). tests/fog-version-release-tag.test.sh builds a fixture repo
with a release tag and a newer archive tag, and fails on the old script
with 'archive/feature-x.2'.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RANCjBL6XSavKYf58MZKZj
…tags-1.5 Take the version base from a release tag only
…atches channel on dev-branch to match commit count since master
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
4 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.
Pull Request for creating the latest stable release from staged security fixes, bug fixes, and minor enhancements in dev-branch. Created as needed or monthly on 11th of every month (queued in gh actions at 11:11 UTC), released once tests are passing