Feature #5769
closed
Can this code be added to system patch package?
0%
Description
The system patch package has a (slight) inconsistent treatment which happens invisibly. When the package is installed and bootup_apply_patches() first called, it looks like patches with local code will be applied but patches with hosted code not already fetched won't be applied until the user manually fetches. The user is not advised and no warning is given that this is skipped for some patches (if hosted code not already held) or that hosted code may be out of date until manually re-fetched.
Is this a sensible point to improve and if so would the rough approach below be ok?
new function to add to patches.inc:
function patch_fetch_all() {
if (!isset($config['installedpackages']['patches']['item']) || !is_array($config['installedpackages']['patches']['item'])) {
return;
}
foreach ($config['installedpackages']['patches']['item'] as $patch) {
if ($patch['id'] && (!isset($patch['patch']) || !is_string($patch['patch']) || $patch['patch'] == '') {
$result = gettext('Patch') . " {$patch['descr']} " . gettext(' not held") . ': ' (patch_fetch($patch['id']) ? gettext("Patch Fetched Successfully") : gettext("Patch Fetch Failed"));
// log this result?
}
}
}
Change to patch_package_install():
function patch_package_install() {
patch_add_shellcmd();
patch_fetch_all(); //NEW LINE ADDED
}The effect would be that any patches where FETCH would be needed (hosted remotely) but no local copy held, would be fetched if possible when the package itself is installed, and therefore be available during bootup_apply_patches() or any other auto-apply. Patches where the remote file is already held would not be re-fetched, this is to prevent the existing version being overwritten without intervention.
If something in this area is sensible, then I have a few quick questions about it:
- Should patches with remote ID be auto-fetched anyway even if existing version is held?
- Should all patches that can be applied, be applied on install following the above fetch, rather than waiting for reboot to do so? (they clearly can be applied other than during early shell since the user can manually apply via GUI but is this always safe or only sometimes?)
- Is the existing test in bootup_apply_patches() correct? It tests both
(!patch_test_revert() && patch_test_apply())to determine if a patch should be auto-applied. But past discussion stated that some patches can be repeatedly applied and reverted, surely this test will prevent any patch being auto-applied if it can be re-applied once applied, or re-reverted once reverted...? - I can't see a cron job to auto-update remote hosted patches to latest versions periodically; if it doesn't exist then should it? (possibly with an 'auto-fetch' flag per patch['item']?)
- What command would be appropriate to log the successful apply/remove/fetch/change of a patch in the system log? (Should log as patches can fundamentally alter system)
Updated by Jim Pingle almost 10 years ago
- Status changed from New to Rejected
1. No. Auto-fetching is a bad idea, especially if it stomps existing content. The contents of the remote patch could have changed, and after fetching they should always be inspected by the user before applying. Otherwise you leave yourself open to exploitation if someone gains control of the server hosting your patches.
2. No, because of the way 2.3 updates the base package is expanded after the reboot so patching before the reboot is a no-op.
3. A patch that can be applied multiple times would fail the first test since it could be reverted if it had already been applied.
4. The package does not (and will not) auto-fetch patches, and see point 1. A cron job doesn't make sense for this package. It's meant to be a simple way to apply patches for development and bug workarounds, not an automated code update/deployment mechanism.
5. In php, use log_error(), from the shell use something such as: /usr/bin/logger -t patches "did something"