From 081fbea14d12f79c8d91ce4fe1f1004c7bc08656 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 12 Apr 2017 17:39:06 +0200 Subject: error in update on Release information changes The value of Origin, Label, Codename and co can be used in user configuration from apts own pinning to unattended upgrades. A repository changing this values can therefore have serious effects on the behaviour of apt and other tools using these values. In a first step we will generate error messages for these changes now explaining the need for explicit confirmation and provide config options and commandline flags to accept them. --- apt-pkg/acquire-item.cc | 60 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 5 deletions(-) (limited to 'apt-pkg/acquire-item.cc') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 574ef4939..f5ff8288b 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1605,13 +1605,63 @@ bool pkgAcqMetaBase::VerifyVendor(string const &) /*{{{*/ if (TransactionManager->MetaIndexParser->CheckDist(ExpectedDist) == false) _error->Warning(_("Conflicting distribution: %s (expected %s but got %s)"), Desc.Description.c_str(), ExpectedDist.c_str(), NowCodename.c_str()); - // might be okay, might be not + + // changed info potentially breaks user config like pinning if (TransactionManager->LastMetaIndexParser != nullptr) { - auto const LastCodename = TransactionManager->LastMetaIndexParser->GetCodename(); - if (LastCodename.empty() == false && NowCodename.empty() == false && LastCodename != NowCodename) - _error->Warning(_("Conflicting distribution: %s (expected %s but got %s)"), - Desc.Description.c_str(), LastCodename.c_str(), NowCodename.c_str()); + auto const AllowInfoChange = _config->FindB("Acquire::AllowReleaseInfoChange", false); + auto const quietInfoChange = _config->FindB("quiet::ReleaseInfoChange", false); + struct { + char const * const Type; + bool const Allowed; + decltype(&metaIndex::GetOrigin) const Getter; + } checkers[] = { + { "Origin", AllowInfoChange, &metaIndex::GetOrigin }, + { "Label", AllowInfoChange, &metaIndex::GetLabel }, + { "Version", true, &metaIndex::GetVersion }, // numbers change all the time, that is okay + { "Suite", AllowInfoChange, &metaIndex::GetSuite }, + { "Codename", AllowInfoChange, &metaIndex::GetCodename }, + { nullptr, false, nullptr } + }; + auto const CheckReleaseInfo = [&](char const * const Type, bool const AllowChange, decltype(checkers[0].Getter) const Getter) { + std::string const Last = (TransactionManager->LastMetaIndexParser->*Getter)(); + std::string const Now = (TransactionManager->MetaIndexParser->*Getter)(); + if (Last == Now) + return true; + auto const Allow = _config->FindB(std::string("Acquire::AllowReleaseInfoChange::").append(Type), AllowChange); + auto const msg = _("Repository '%s' changed its '%s' value from '%s' to '%s'"); + if (Allow == false) + _error->Error(msg, Desc.Description.c_str(), Type, Last.c_str(), Now.c_str()); + else if (_config->FindB(std::string("quiet::ReleaseInfoChange::").append(Type), quietInfoChange) == false) + _error->Notice(msg, Desc.Description.c_str(), Type, Last.c_str(), Now.c_str()); + return Allow; + }; + bool CRI = true; + for (short i = 0; checkers[i].Type != nullptr; ++i) + if (CheckReleaseInfo(checkers[i].Type, checkers[i].Allowed, checkers[i].Getter) == false) + CRI = false; + + { + auto const Last = TransactionManager->LastMetaIndexParser->GetDefaultPin(); + auto const Now = TransactionManager->MetaIndexParser->GetDefaultPin(); + if (Last != Now) + { + auto const Allow = _config->FindB("Acquire::AllowReleaseInfoChange::DefaultPin", AllowInfoChange); + auto const msg = _("Repository '%s' changed its default priority for %s from %hi to %hi."); + if (Allow == false) + _error->Error(msg, Desc.Description.c_str(), "apt_preferences(5)", Last, Now); + else if (_config->FindB("quiet::ReleaseInfoChange::DefaultPin", quietInfoChange) == false) + _error->Notice(msg, Desc.Description.c_str(), "apt_preferences(5)", Last, Now); + CRI &= Allow; + } + } + if (CRI == false) + { + // TRANSLATOR: %s is the name of the manpage in question, e.g. apt-secure(8) + _error->Notice(_("This must be accepted explicitly before updates for " + "this repository can be applied. See %s manpage for details."), "apt-secure(8)"); + return false; + } } return true; } -- cgit v1.2.3 From 96ebab48c25fcd1ee83729cdba4be8a6343a8766 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 28 May 2017 13:24:33 +0200 Subject: show a Release-Notes URI if infos were changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This gives the repository owner a chance to explain why this change was needed – e.g. explaining the organisational changes or simply detailing the changes in the new release made. Note that this URI is also shown if the change is accepted, so it also draws attention to release notes of minor updates (if users watch apt output closely). --- apt-pkg/acquire-item.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'apt-pkg/acquire-item.cc') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index f5ff8288b..ddcff5808 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1623,6 +1623,7 @@ bool pkgAcqMetaBase::VerifyVendor(string const &) /*{{{*/ { "Codename", AllowInfoChange, &metaIndex::GetCodename }, { nullptr, false, nullptr } }; + _error->PushToStack(); auto const CheckReleaseInfo = [&](char const * const Type, bool const AllowChange, decltype(checkers[0].Getter) const Getter) { std::string const Last = (TransactionManager->LastMetaIndexParser->*Getter)(); std::string const Now = (TransactionManager->MetaIndexParser->*Getter)(); @@ -1655,6 +1656,16 @@ bool pkgAcqMetaBase::VerifyVendor(string const &) /*{{{*/ CRI &= Allow; } } + if (_error->empty(GlobalError::NOTICE) == false) + { + auto const notes = TransactionManager->MetaIndexParser->GetReleaseNotes(); + if (notes.empty() == false) + { + // TRANSLATOR: the "this" refers to changes in the repository like a new release or owner change + _error->Notice(_("More information about this can be found online in the Release notes at: %s"), notes.c_str()); + } + } + _error->MergeWithStack(); if (CRI == false) { // TRANSLATOR: %s is the name of the manpage in question, e.g. apt-secure(8) -- cgit v1.2.3 From ca8da1bf83ecc90ba882520b79c1cda03ee7485d Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 28 May 2017 16:55:45 +0200 Subject: allow frontends to override releaseinfo change behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Having messages being printed on the error stack and confirm them by commandline flags is an okayish first step, but some frontends will probably want to have a more interactive feeling here with a proper question the user can just press yes/no for as for some frontends a commandline flag makes no sense… --- apt-pkg/acquire-item.cc | 61 ++++++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 28 deletions(-) (limited to 'apt-pkg/acquire-item.cc') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index ddcff5808..975116d1a 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1609,6 +1609,7 @@ bool pkgAcqMetaBase::VerifyVendor(string const &) /*{{{*/ // changed info potentially breaks user config like pinning if (TransactionManager->LastMetaIndexParser != nullptr) { + std::vector Changes; auto const AllowInfoChange = _config->FindB("Acquire::AllowReleaseInfoChange", false); auto const quietInfoChange = _config->FindB("quiet::ReleaseInfoChange", false); struct { @@ -1623,24 +1624,21 @@ bool pkgAcqMetaBase::VerifyVendor(string const &) /*{{{*/ { "Codename", AllowInfoChange, &metaIndex::GetCodename }, { nullptr, false, nullptr } }; - _error->PushToStack(); auto const CheckReleaseInfo = [&](char const * const Type, bool const AllowChange, decltype(checkers[0].Getter) const Getter) { std::string const Last = (TransactionManager->LastMetaIndexParser->*Getter)(); std::string const Now = (TransactionManager->MetaIndexParser->*Getter)(); if (Last == Now) - return true; - auto const Allow = _config->FindB(std::string("Acquire::AllowReleaseInfoChange::").append(Type), AllowChange); - auto const msg = _("Repository '%s' changed its '%s' value from '%s' to '%s'"); - if (Allow == false) - _error->Error(msg, Desc.Description.c_str(), Type, Last.c_str(), Now.c_str()); - else if (_config->FindB(std::string("quiet::ReleaseInfoChange::").append(Type), quietInfoChange) == false) - _error->Notice(msg, Desc.Description.c_str(), Type, Last.c_str(), Now.c_str()); - return Allow; + return; + auto const Allow = _config->FindB(std::string("Acquire::AllowReleaseInfoChange::").append(Type), AllowChange); + if (Allow == true && _config->FindB(std::string("quiet::ReleaseInfoChange::").append(Type), quietInfoChange) == true) + return; + std::string msg; + strprintf(msg, _("Repository '%s' changed its '%s' value from '%s' to '%s'"), + Desc.Description.c_str(), Type, Last.c_str(), Now.c_str()); + Changes.push_back({Type, std::move(Last), std::move(Now), std::move(msg), Allow}); }; - bool CRI = true; for (short i = 0; checkers[i].Type != nullptr; ++i) - if (CheckReleaseInfo(checkers[i].Type, checkers[i].Allowed, checkers[i].Getter) == false) - CRI = false; + CheckReleaseInfo(checkers[i].Type, checkers[i].Allowed, checkers[i].Getter); { auto const Last = TransactionManager->LastMetaIndexParser->GetDefaultPin(); @@ -1648,31 +1646,38 @@ bool pkgAcqMetaBase::VerifyVendor(string const &) /*{{{*/ if (Last != Now) { auto const Allow = _config->FindB("Acquire::AllowReleaseInfoChange::DefaultPin", AllowInfoChange); - auto const msg = _("Repository '%s' changed its default priority for %s from %hi to %hi."); - if (Allow == false) - _error->Error(msg, Desc.Description.c_str(), "apt_preferences(5)", Last, Now); - else if (_config->FindB("quiet::ReleaseInfoChange::DefaultPin", quietInfoChange) == false) - _error->Notice(msg, Desc.Description.c_str(), "apt_preferences(5)", Last, Now); - CRI &= Allow; + if (Allow == false || _config->FindB("quiet::ReleaseInfoChange::DefaultPin", quietInfoChange) == false) + { + std::string msg; + strprintf(msg, _("Repository '%s' changed its default priority for %s from %hi to %hi."), + Desc.Description.c_str(), "apt_preferences(5)", Last, Now); + Changes.push_back({"DefaultPin", std::to_string(Last), std::to_string(Now), std::move(msg), Allow}); + } } } - if (_error->empty(GlobalError::NOTICE) == false) + if (Changes.empty() == false) { auto const notes = TransactionManager->MetaIndexParser->GetReleaseNotes(); if (notes.empty() == false) { + std::string msg; // TRANSLATOR: the "this" refers to changes in the repository like a new release or owner change - _error->Notice(_("More information about this can be found online in the Release notes at: %s"), notes.c_str()); + strprintf(msg, _("More information about this can be found online in the Release notes at: %s"), notes.c_str()); + Changes.push_back({"Release-Notes", "", std::move(notes), std::move(msg), true}); } + if (std::any_of(Changes.begin(),Changes.end(),[](pkgAcquireStatus::ReleaseInfoChange const &c) { return c.DefaultAction == false; })) + { + std::string msg; + // TRANSLATOR: %s is the name of the manpage in question, e.g. apt-secure(8) + strprintf(msg, _("This must be accepted explicitly before updates for " + "this repository can be applied. See %s manpage for details."), "apt-secure(8)"); + Changes.push_back({"Confirmation", "", "", std::move(msg), true}); + } + } - _error->MergeWithStack(); - if (CRI == false) - { - // TRANSLATOR: %s is the name of the manpage in question, e.g. apt-secure(8) - _error->Notice(_("This must be accepted explicitly before updates for " - "this repository can be applied. See %s manpage for details."), "apt-secure(8)"); - return false; - } + if (Owner->Log == nullptr) + return pkgAcquireStatus::ReleaseInfoChangesAsGlobalErrors(std::move(Changes)); + return Owner->Log->ReleaseInfoChanges(TransactionManager->LastMetaIndexParser, TransactionManager->MetaIndexParser, std::move(Changes)); } return true; } -- cgit v1.2.3