Bug 229689 - freebsd-update shouldn't add system breaking comments to config files
Summary: freebsd-update shouldn't add system breaking comments to config files
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 11.2-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2018-07-11 06:49 UTC by Dan MacDonald
Modified: 2023-09-19 22:44 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dan MacDonald 2018-07-11 06:49:03 UTC
Yesterday I used freebsd-update to upgrade from 11.1 to 11.2. I probably should've waited until the weekend or a time when I wasn't as tired or in a hurry because it was past my bedtime on a school night and so after it had downloaded all the patches and it was asking me to evaluate the changes in various config files I just said "Yeah whatever" and blindly accepted all of the suggested changes, presuming that freebsd-update or any of the update scripts wouldn't do anything stupid but unfortunately it did make some breaking changes to my config files to prevented me logging in after the upgrade.

I'm unsure of the full extent of the damage but the main ones I'm aware of were a couple of 'uncommented comments' added to /etc/ssh/sshd_config. freebsd-update added these two lines:

<<<<<<< current version
>>>>>>> 11.2-RELEASE

Because there was no hash sign at the beginning of either line, sshd choked on these two lines and I couldn't log into my server via ssh. I then had to spend 30 minutes digging out a VGA cable and hooking up a monitor to fix my ssh config file. Thankfully the server is in my house and wasn't doing anything important at the time but I can see this biting others harder and in a more expensive way. I can guarantee I won't be the only person who blindly accepts the suggested modifications during an upgrade.
Comment 1 Thanos 2018-07-20 11:37:00 UTC
MARKED AS SPAM
Comment 2 Graham Perrin freebsd_committer freebsd_triage 2021-12-19 17:05:47 UTC
(In reply to Dan MacDonald from comment #0)

> … no hash sign at the beginning …

Good point. A design flaw, IMHO.
Comment 3 Graham Perrin freebsd_committer freebsd_triage 2021-12-19 17:07:50 UTC
(In reply to Dan MacDonald from comment #0)


> … tired or in a hurry … blindly accepted …

Somewhat off-topic: 

* might oversight (blind acceptance) have been less likely if 
  something other than vi had been used? 

<https://github.com/yangzhong-freebsd/lua-httpd/issues/3> was notionally in the context of a proof-of-concept installer (see <https://freebsdfoundation.org/blog/technology-roadmap/>), however the general principle – allowing the end user to prefer ee – might extend to situations such as those mentioned in this bug 229689.
Comment 4 Ed Maste freebsd_committer freebsd_triage 2022-12-14 17:39:02 UTC
(In reply to Graham Perrin from comment #2)
Prefixing the conflict markers with a comment character (#) isn't viable - it could result in even more confusing or unclear behaviour, as the config file would end up with a mix of potentially conflicting settings from the user's config and updated config.

When freebsd-update encounters this case it prints

---
The following file could not be merged automatically: ${F}
Press Enter to edit this file in ${EDITOR} and resolve the conflicts
manually...
---

and it is up to the user to resolve the conflicts.

See https://reviews.freebsd.org/D37703 for a proposal to inform the user if they have unresolved conflicts (and return to editing the file).
Comment 5 Graham Perrin freebsd_committer freebsd_triage 2022-12-15 00:40:45 UTC
(In reply to Ed Maste from comment #4)

Thanks (OT, a few months ago I discovered _why_ things such as 
<<<<<<< are used …)
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-03-02 01:22:04 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ceb5f28ba5fcfa69de7410d2327d4a5abf2a421f

commit ceb5f28ba5fcfa69de7410d2327d4a5abf2a421f
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2022-12-14 17:34:59 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-03-02 01:20:33 +0000

    freebsd-update: re-edit files if merge conflict markers remain

    freebsd-update will open ${EDITOR} if conflicts occur while merging
    updates to config files.  Inform the user if they've left conflict
    markers behind, and go back to editing the file.

    PR:             185546
    PR:             229689
    Reviewed by:    delphij
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D37703

 usr.sbin/freebsd-update/freebsd-update.sh | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2023-03-02 18:32:03 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c7e3703352037a5afacdc4126725f351fe7da72b

commit c7e3703352037a5afacdc4126725f351fe7da72b
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2022-12-14 17:34:59 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-03-02 18:29:57 +0000

    freebsd-update: re-edit files if merge conflict markers remain

    freebsd-update will open ${EDITOR} if conflicts occur while merging
    updates to config files.  Inform the user if they've left conflict
    markers behind, and go back to editing the file.

    PR:             185546
    PR:             229689
    Reviewed by:    delphij
    Approved by:    re (cperciva, expedited MFC)
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D37703

    (cherry picked from commit ceb5f28ba5fcfa69de7410d2327d4a5abf2a421f)

 usr.sbin/freebsd-update/freebsd-update.sh | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2023-03-02 20:51:30 UTC
A commit in branch releng/13.2 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a6c6edbe9d37caf1dbc81f918adf8c1f451a534e

commit a6c6edbe9d37caf1dbc81f918adf8c1f451a534e
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2022-12-14 17:34:59 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-03-02 20:50:57 +0000

    freebsd-update: re-edit files if merge conflict markers remain

    freebsd-update will open ${EDITOR} if conflicts occur while merging
    updates to config files.  Inform the user if they've left conflict
    markers behind, and go back to editing the file.

    PR:             185546
    PR:             229689
    Reviewed by:    delphij
    Approved by:    re (cperciva)
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D37703

    (cherry picked from commit ceb5f28ba5fcfa69de7410d2327d4a5abf2a421f)
    (cherry picked from commit c7e3703352037a5afacdc4126725f351fe7da72b)

 usr.sbin/freebsd-update/freebsd-update.sh | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
Comment 9 Graham Perrin freebsd_committer freebsd_triage 2023-03-03 06:20:56 UTC
(In reply to commit-hook from comment #8)

Should we have a 13.2 release note for a6c6edbe9d37caf1dbc81f918adf8c1f451a534e?
Comment 10 Ed Maste freebsd_committer freebsd_triage 2023-03-06 17:09:05 UTC
(In reply to Graham Perrin from comment #9)
It wouldn't hurt, but also is probably not going to be that useful to people upgrading *to* 13.2, who are upgrading *from* a FreeBSD version where freebsd-update does not yet have this change.
Comment 11 commit-hook freebsd_committer freebsd_triage 2023-06-21 12:58:53 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=bb727917364f7ded1d24f599389288c63b23d862

commit bb727917364f7ded1d24f599389288c63b23d862
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2022-12-14 17:34:59 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-06-21 12:58:01 +0000

    freebsd-update: re-edit files if merge conflict markers remain

    freebsd-update will open ${EDITOR} if conflicts occur while merging
    updates to config files.  Inform the user if they've left conflict
    markers behind, and go back to editing the file.

    PR:             185546
    PR:             229689
    Reviewed by:    delphij
    Approved by:    re (cperciva, expedited MFC)
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D37703

    (cherry picked from commit ceb5f28ba5fcfa69de7410d2327d4a5abf2a421f)
    (cherry picked from commit c7e3703352037a5afacdc4126725f351fe7da72b)
    (cherry picked from commit e27ded83c76a609687a3d9e82b80fe7e1b782bf6)
    (cherry picked from commit b562307b70346030f59fe6a05d125814c74da47b)

 usr.sbin/freebsd-update/freebsd-update.sh | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)