Skip to content

fix(build.func): stop a modified conffile from aborting the whole update - #16438

Open
MarcvsTvllivs wants to merge 1 commit into
community-scripts:mainfrom
MarcvsTvllivs:fix/update-conffile-guard
Open

fix(build.func): stop a modified conffile from aborting the whole update#16438
MarcvsTvllivs wants to merge 1 commit into
community-scripts:mainfrom
MarcvsTvllivs:fix/update-conffile-guard

Conversation

@MarcvsTvllivs

Copy link
Copy Markdown

✍️ Description

update_script() runs with no tty. When an update pulls a package whose conffile the operator has
edited, and the new version also ships a changed copy of that file, dpkg raises its "modified
since installation" prompt, finds nothing to read it, and fails.

The consequence is larger than the one package:

  1. dpkg exits with end of file on stdin at conffile prompt
  2. the package is left half configured, state iU
  3. apt returns 100, so every other pending package on that container also stops updating
  4. the surfaced error is APT: Package manager error (broken packages / dependency problems),
    which never mentions conffiles and points at the wrong thing
  5. the built in recovery, dpkg --configure -a at misc/build.func and misc/tools.func, runs
    without --force-conf* and so hits the identical prompt and cannot clear it

Point 3 is the reason this is worth fixing rather than documenting. The container keeps reporting
itself as a normal install while silently accumulating unpatched packages. I found it during a
sweep of 39 containers: one edited conffile had frozen an entire container's package set, and
nothing surfaced it.

Real example, ntfy (installed from archive.ntfy.sh/apt/, where /etc/ntfy/server.yml is a
registered conffile). Edit it to enable auth, then update once 2.27.0 ships its own change to it:

Configuration file '/etc/ntfy/server.yml'
 ==> Modified (by you or by a script) since installation.
 ==> Package distributor has shipped an updated version.
*** server.yml (Y/I/N/O/D/Z) [default=N] ? dpkg: error processing package ntfy (--configure):
 end of file on stdin at conffile prompt
Errors were encountered while processing:
 ntfy

[ERROR] in line 48: exit code 100 (APT: Package manager error (broken packages / dependency
problems)): while executing command apt upgrade -y

Why this is fixed centrally rather than per script

87 of 583 ct/*.sh run an apt upgrade inside update_script(), and none of them pass any
Dpkg::Options. Rather than edit 87 files and leave every future script free to reintroduce it,
this hooks the single dispatch in start() that all of them already funnel through, next to the
existing check_container_os_guard pre-flight. Three call sites, one function block.

Count reproducible with:

for f in ct/*.sh; do
  sed -n '/^function update_script/,/^}/p' "$f" \
    | grep -E "apt(-get)?[[:space:]]+(-[a-z][^ ]*[[:space:]]+)*(dist-upgrade|full-upgrade|upgrade)" \
    | grep -qv "Dpkg::Options" && basename "$f"
done | wc -l

Only .deb packaged apps can actually hit this, since apps unpacked into /opt have no conffiles,
so the real exposure is a subset of the 87.

What the change does

apt_conffile_guard_begin writes /etc/apt/apt.conf.d/99-community-scripts-conffile with
--force-confdef --force-confold, and apt_conffile_guard_end removes it again.

Two deliberate choices worth flagging for review:

It is scoped to the update run, not permanent. The drop-in is deleted afterwards, so an
operator's own later apt calls behave exactly as before. This seemed better than changing apt's
behaviour for the life of the container.

It reports divergence rather than only silencing it. Suppressing the prompt alone would trade a
loud failure for a silent one, which is the fair objection to this change. So the guard records
which .dpkg-dist / .dpkg-new files existed before, and afterwards warns about any that are
new:

Kept your version of these config files; upstream shipped changes alongside them:
  /etc/ntfy/server.yml (new upstream version: /etc/ntfy/server.yml.dpkg-dist)

Pre-existing ones are not re-reported on every subsequent update.

--force-confold is the right default here: silently overwriting an operator's configuration would
be worse than the current failure. The cost is that a container keeps its old file when upstream
adds a genuinely new option, which is exactly what the warning above exists to surface. This is the
same tradeoff Debian's own unattended-upgrades makes.

Note for maintainers

If you would rather have an explicit opt-in helper (an apt_upgrade_safe() beside the existing
apt_update_safe() in misc/core.func) than a config drop-in, I am happy to rework it that way.
That route is more explicit but needs 87 call site edits and leaves future scripts able to
reintroduce the bug, which is why I went central first. Your call.

Testing

bash -n on the patched misc/build.func passes on bash 5.2. (Note it does not parse on bash 3.2
either before or after this change, because of the pre-existing [[ -v NEW["$k"] ]] at line 1719.)

Functional test of both guards, run inside real containers on a Proxmox 9.2 node:

# Check Result
1 no drop-in present beforehand PASS
2 drop-in written by _begin PASS
3 apt-config dump shows --force-confdef / --force-confold in effect PASS
4 baseline picks up a real pre-existing .dpkg-dist PASS
5 a newly appearing .dpkg-dist is reported PASS
6 the pre-existing one is not re-reported PASS
7 drop-in removed by _end PASS
8 apt-config dump back to no Dpkg::Options PASS
9 no-ops cleanly on a non-apt container (Alpine) PASS
10 writes nothing on a non-apt container PASS

🔗 Related Issue

No existing issue. Searched open and closed issues for "force-confold", "confold" and "conffile"
and found no match, so this is filed directly as a PR. Happy to open an issue first instead if you
prefer that order.

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards.
  • Tested thoroughly – Changes work as expected.
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.

🤖 AI Assistance (X in brackets)

  • AI was used – written with Claude Opus 5 (max reasoning effort), then reviewed and tested
    by me on a live Proxmox 9.2 node before submitting.

    One clarification in the interest of not ticking a box loosely: AGENTS.md and
    pve-script-creator.agent.md are guidance for authoring ct/install scripts, and this PR
    touches neither. It is a change to misc/build.func, so those documents did not apply. Flagging
    it rather than claiming conformance I did not have to meet.


🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing functionality in a way that may require updates.
  • 🆕 New script – A fully functional and tested script or script set.
  • 🌍 Website update – Changes to script metadata (PocketBase/website data).
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, AppName.md, CONTRIBUTING.md, or other docs.

update_script() runs with no tty. If an update pulls a package whose conffile
the operator edited, and the new version also ships a changed copy, dpkg raises
its "modified since installation" prompt, finds nothing to read it, and exits
with "end of file on stdin at conffile prompt".

The package is then left half configured (iU) and apt returns 100, which aborts
every other pending package on that container, not just the one being updated.
The error surfaced is "APT: Package manager error (broken packages / dependency
problems)", which does not mention conffiles. The dpkg --configure -a recovery
paths run without --force-conf* and hit the same prompt, so they cannot clear
it either.

87 of 583 ct scripts run an apt upgrade inside update_script() and none pass
Dpkg::Options, so this hooks the single dispatch in start() that all of them
already funnel through, beside the existing check_container_os_guard.

The guard is scoped to the update run and removed afterwards, so an operator's
own later apt calls are unaffected. It also reports any newly appearing
.dpkg-dist / .dpkg-new files, so keeping the local version stays visible
instead of silently diverging from upstream.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants