FdoSecrets: Improve client executable path handling (#6915)

* Fixes #6459 

Improves the overall handling of FdoSecrets showing client executable paths to the user. It does the following:

* Check executable file existence as described in [RFC] fdosecrets: add optional confirmation to secret access (#4733)
* Show application PID and dbus address in the client list
* When the executable file is inaccessible, depending on where the client name is shown:
    * when shown inline, e.g. in notification text, where space is limited, clearly say that the path is invalid
    * when shown in auth dialog, show warning and print detailed info about the client
    * when shown in the client list, draw a warning icon

Co-authored-by: Jonathan White <support@dmapps.us>

share/translations/keepassxc_en.ts

@@ -66,10 +66,6 @@
         <source>Remember</source>
         <translation type="unfinished"></translation>
     </message>
-    <message>
-        <source>Allow access to entries</source>
-        <translation type="unfinished"></translation>
-    </message>
     <message>
         <source>Allow Selected</source>
         <translation type="unfinished"></translation>
@@ -78,6 +74,30 @@
         <source>Deny All</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>Non-existing/inaccessible executable path. Please double-check the client is legit.</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Name</source>
+        <translation type="unfinished">Name</translation>
+    </message>
+    <message>
+        <source>PID</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Executable</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Command Line</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Details</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>AccessControlDialog::DenyButton</name>
@@ -3773,28 +3793,16 @@ Error: %1</source>
 <context>
     <name>FdoSecrets::SettingsClientModel</name>
     <message>
-        <source>Application</source>
+        <source>Unknown</source>
-        <translation type="unfinished"></translation>
+        <translation type="unfinished">Unknown</translation>
     </message>
     <message>
-        <source>Manage</source>
+        <source>Non-existing/inaccessible executable path. Please double-check the client is legit.</source>
         <translation type="unfinished"></translation>
     </message>
 </context>
 <context>
     <name>FdoSecrets::SettingsDatabaseModel</name>
-    <message>
-        <source>File Name</source>
-        <translation type="unfinished"></translation>
-    </message>
-    <message>
-        <source>Group</source>
-        <translation type="unfinished">Group</translation>
-    </message>
-    <message>
-        <source>Manage</source>
-        <translation type="unfinished"></translation>
-    </message>
     <message>
         <source>Unlock to show</source>
         <translation type="unfinished"></translation>
@@ -7249,6 +7257,14 @@ Please consider generating a new key file.</source>
         <source>Please present or touch your YubiKey to continue…</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>unknown executable (DBus address %1)</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>%1 (invalid executable path)</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>QtIOCompressor</name>
@@ -7724,6 +7740,40 @@ Please consider generating a new key file.</source>
         <translation type="unfinished"></translation>
     </message>
 </context>
+<context>
+    <name>SettingsClientModel</name>
+    <message>
+        <source>Application</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>PID</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>DBus Address</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Manage</source>
+        <translation type="unfinished"></translation>
+    </message>
+</context>
+<context>
+    <name>SettingsDatabaseModel</name>
+    <message>
+        <source>File Name</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Group</source>
+        <translation type="unfinished">Group</translation>
+    </message>
+    <message>
+        <source>Manage</source>
+        <translation type="unfinished"></translation>
+    </message>
+</context>
 <context>
     <name>SettingsWidgetFdoSecrets</name>
     <message>

src/fdosecrets/dbus/DBusClient.cpp

@@ -22,14 +22,53 @@
 #include "fdosecrets/dbus/DBusMgr.h"
 #include "fdosecrets/objects/SessionCipher.h"
 
+#include <utility>
+
 namespace FdoSecrets
 {
-    DBusClient::DBusClient(DBusMgr* dbus, const QString& address, uint pid, const QString& name)
+    bool ProcInfo::operator==(const ProcInfo& other) const
-        : m_dbus(dbus)
-        , m_address(address)
-        , m_pid(pid)
-        , m_name(name)
     {
+        return this->pid == other.pid && this->ppid == other.ppid && this->exePath == other.exePath
+               && this->name == other.name && this->command == other.command;
+    }
+
+    bool ProcInfo::operator!=(const ProcInfo& other) const
+    {
+        return !(*this == other);
+    }
+
+    bool PeerInfo::operator==(const PeerInfo& other) const
+    {
+        return this->address == other.address && this->pid == other.pid && this->valid == other.valid
+               && this->hierarchy == other.hierarchy;
+    }
+
+    bool PeerInfo::operator!=(const PeerInfo& other) const
+    {
+        return !(*this == other);
+    }
+
+    DBusClient::DBusClient(DBusMgr* dbus, PeerInfo process)
+        : m_dbus(dbus)
+        , m_process(std::move(process))
+    {
+    }
+
+    DBusMgr* DBusClient::dbus() const
+    {
+        return m_dbus;
+    }
+
+    QString DBusClient::name() const
+    {
+        auto exePath = m_process.exePath();
+        if (exePath.isEmpty()) {
+            return QObject::tr("unknown executable (DBus address %1)").arg(m_process.address);
+        }
+        if (!m_process.valid) {
+            return QObject::tr("%1 (invalid executable path)").arg(exePath);
+        }
+        return exePath;
     }
 
     bool DBusClient::itemKnown(const QUuid& uuid) const

src/fdosecrets/dbus/DBusClient.h

@@ -30,6 +30,56 @@ namespace FdoSecrets
     class DBusMgr;
     class CipherPair;
 
+    struct ProcInfo
+    {
+        uint pid;
+        uint ppid;
+        QString exePath;
+        QString name;
+        QString command;
+
+        bool operator==(const ProcInfo& other) const;
+        bool operator!=(const ProcInfo& other) const;
+    };
+
+    /**
+     * Contains info representing a process.
+     * This can be obtained by DBusMgr::serviceInfo given a dbus address.
+     */
+    struct PeerInfo
+    {
+        /**
+         * @brief DBus address
+         */
+        QString address;
+
+        uint pid;
+        /**
+         * @brief Whether current process' exePath points to a valid executable file.
+         *
+         * Note that an empty exePath is not valid.
+         */
+        bool valid;
+
+        /**
+         * @brief List of parents of the process.
+         *
+         * The first element is the current process. The last element is usually PID 1.
+         *
+         * This is for showing to the user only and is intentionally simple.
+         * Getting detailed process info is beyond the scope of KPXC.
+         */
+        QList<ProcInfo> hierarchy;
+
+        QString exePath() const
+        {
+            return hierarchy.front().exePath;
+        }
+
+        bool operator==(const PeerInfo& other) const;
+        bool operator!=(const PeerInfo& other) const;
+    };
+
     /**
      * Represent a client that has made requests to our service. A client is identified by its
      * DBus address, which is guaranteed to be unique by the DBus protocol.
@@ -48,30 +98,23 @@ namespace FdoSecrets
         /**
          * @brief Given peer's service address, construct a client object
          * @param address obtained from `QDBusMessage::service()`
-         * @param pid the process PID
+         * @param process the process info
-         * @param name the process name
          */
-        explicit DBusClient(DBusMgr* dbus, const QString& address, uint pid, const QString& name);
+        explicit DBusClient(DBusMgr* dbus, PeerInfo process);
 
-        DBusMgr* dbus() const
+        DBusMgr* dbus() const;
-        {
-            return m_dbus;
-        }
 
         /**
          * @return The human readable client name, usually the process name
          */
-        QString name() const
+        QString name() const;
-        {
-            return m_name;
-        }
 
         /**
          * @return The unique DBus address of the client
          */
         QString address() const
         {
-            return m_address;
+            return m_process.address;
         }
 
         /**
@@ -79,7 +122,15 @@ namespace FdoSecrets
          */
         uint pid() const
         {
-            return m_pid;
+            return m_process.pid;
+        }
+
+        /**
+         * @return The process info
+         */
+        const PeerInfo& processInfo() const
+        {
+            return m_process;
         }
 
         QSharedPointer<CipherPair>
@@ -123,10 +174,7 @@ namespace FdoSecrets
 
     private:
         QPointer<DBusMgr> m_dbus;
-        QString m_address;
+        PeerInfo m_process;
-
-        uint m_pid{0};
-        QString m_name{};
 
         bool m_authorizedAll{false};
 

src/fdosecrets/dbus/DBusMgr.cpp

@@ -167,6 +167,46 @@ namespace FdoSecrets
 </interface>
 )xml";
 
+    // read /proc, each field except pid may be empty
+    ProcInfo readProc(uint pid)
+    {
+        ProcInfo info{};
+        info.pid = pid;
+
+        // The /proc/pid/exe link is more reliable than /proc/pid/cmdline
+        // It's still weak and if the application does a prctl(PR_SET_DUMPABLE, 0) this link cannot be accessed.
+        QFileInfo exe(QStringLiteral("/proc/%1/exe").arg(pid));
+        info.exePath = exe.canonicalFilePath();
+        qDebug() << "process" << pid << info.exePath;
+
+        // /proc/pid/cmdline gives full command line
+        QFile cmdline(QStringLiteral("/proc/%1/cmdline").arg(pid));
+        if (cmdline.open(QFile::ReadOnly)) {
+            info.command = QString::fromLocal8Bit(cmdline.readAll().replace('\0', ' ')).trimmed();
+        }
+        qDebug() << "process" << pid << info.command;
+
+        // /proc/pid/stat gives ppid, name
+        QFile stat(QStringLiteral("/proc/%1/stat").arg(pid));
+        if (stat.open(QIODevice::ReadOnly)) {
+            auto line = stat.readAll();
+            // find comm field without looking in what's inside as it's user controlled
+            auto commStart = line.indexOf('(');
+            auto commEnd = line.lastIndexOf(')');
+            if (commStart != -1 && commEnd != -1 && commStart < commEnd) {
+                // start past '(', and subtract 2 from total length
+                info.name = QString::fromLocal8Bit(line.mid(commStart + 1, commEnd + 1 - commStart - 2));
+
+                auto parts = line.mid(commEnd + 1).trimmed().split(' ');
+                if (parts.size() >= 2) {
+                    info.ppid = parts[1].toUInt();
+                }
+            }
+        }
+
+        return info;
+    }
+
     DBusMgr::DBusMgr()
         : m_conn(QDBusConnection::sessionBus())
     {
@@ -198,18 +238,33 @@ namespace FdoSecrets
         return m_clients.values();
     }
 
-    bool DBusMgr::serviceInfo(const QString& addr, ProcessInfo& info) const
+    bool DBusMgr::serviceInfo(const QString& addr, PeerInfo& info) const
     {
+        info.address = addr;
         auto pid = m_conn.interface()->servicePid(addr);
         if (!pid.isValid()) {
             return false;
         }
         info.pid = pid.value();
-        // The /proc/pid/exe link is more reliable than /proc/pid/cmdline
-        // It's still weak and if the application does a prctl(PR_SET_DUMPABLE, 0) this link cannot be accessed.
-        QFileInfo proc(QStringLiteral("/proc/%1/exe").arg(pid.value()));
-        info.exePath = proc.canonicalFilePath();
 
+        // get the whole hierarchy
+        uint ppid = info.pid;
+        while (ppid != 0) {
+            auto proc = readProc(ppid);
+            info.hierarchy.append(proc);
+            ppid = proc.ppid;
+        }
+
+        // check if the exe file is valid
+        // (assuming for now that an accessible file is valid)
+        info.valid = QFileInfo::exists(info.exePath());
+
+        // ask again to double-check and protect against pid reuse
+        auto newPid = m_conn.interface()->servicePid(addr);
+        if (!newPid.isValid() || newPid.value() != pid.value()) {
+            // either the peer already gone or the pid changed to something else
+            return false;
+        }
         return true;
     }
 
@@ -321,11 +376,11 @@ namespace FdoSecrets
         auto pidStr = tr("Unknown", "Unknown PID");
         auto exeStr = tr("Unknown", "Unknown executable path");
 
-        ProcessInfo info{};
+        PeerInfo info{};
         if (serviceInfo(DBUS_SERVICE_SECRET, info)) {
             pidStr = QString::number(info.pid);
-            if (!info.exePath.isEmpty()) {
+            if (!info.exePath().isEmpty()) {
-                exeStr = info.exePath;
+                exeStr = info.exePath();
             }
         }
 
@@ -560,23 +615,17 @@ namespace FdoSecrets
             }
             it = m_clients.insert(addr, client);
         }
-        // double check the client
-        ProcessInfo info{};
-        if (!serviceInfo(addr, info) || info.pid != it.value()->pid()) {
-            dbusServiceUnregistered(addr);
-            return {};
-        }
         return it.value();
     }
 
     DBusClientPtr DBusMgr::createClient(const QString& addr)
     {
-        ProcessInfo info{};
+        PeerInfo info{};
         if (!serviceInfo(addr, info)) {
             return {};
         }
 
-        auto client = DBusClientPtr(new DBusClient(this, addr, info.pid, info.exePath.isEmpty() ? addr : info.exePath));
+        auto client = DBusClientPtr(new DBusClient(this, info));
 
         emit clientConnected(client);
         m_watcher.addWatchedService(addr);
@@ -612,6 +661,7 @@ namespace FdoSecrets
         }
         auto client = it.value();
 
+        // client will notify DBusMgr and call DBusMgr::removeClient
         client->disconnectDBus();
     }
 } // namespace FdoSecrets

src/fdosecrets/dbus/DBusMgr.h

@@ -219,12 +219,7 @@ namespace FdoSecrets
     private:
         QDBusConnection m_conn;
 
-        struct ProcessInfo
+        bool serviceInfo(const QString& addr, PeerInfo& info) const;
-        {
-            uint pid;
-            QString exePath;
-        };
-        bool serviceInfo(const QString& addr, ProcessInfo& info) const;
 
         bool sendDBusSignal(const QString& path,
                             const QString& interface,

src/fdosecrets/objects/Prompt.cpp

@@ -291,7 +291,8 @@ namespace FdoSecrets
         }
         if (!entries.isEmpty()) {
             QString app = tr("%1 (PID: %2)").arg(client->name()).arg(client->pid());
-            auto ac = new AccessControlDialog(findWindow(m_windowId), entries, app, AuthOption::Remember);
+            auto ac = new AccessControlDialog(
+                findWindow(m_windowId), entries, app, client->processInfo(), AuthOption::Remember);
             connect(ac, &AccessControlDialog::finished, this, &UnlockPrompt::itemUnlockFinished);
             connect(ac, &AccessControlDialog::finished, ac, &AccessControlDialog::deleteLater);
             ac->open();

src/fdosecrets/widgets/AccessControlDialog.cpp

@@ -20,6 +20,7 @@
 #include "AccessControlDialog.h"
 #include "ui_AccessControlDialog.h"
 
+#include "fdosecrets/dbus/DBusClient.h"
 #include "fdosecrets/widgets/RowButtonHelper.h"
 
 #include "core/Entry.h"
@@ -31,9 +32,12 @@
 AccessControlDialog::AccessControlDialog(QWindow* parent,
                                          const QList<Entry*>& entries,
                                          const QString& app,
+                                         const FdoSecrets::PeerInfo& info,
                                          AuthOptions authOptions)
     : m_ui(new Ui::AccessControlDialog())
+    , m_rememberCheck()
     , m_model(new EntryModel(entries))
+    , m_decisions()
 {
     if (parent) {
         // Force the creation of the QWindow, without this windowHandle() will return nullptr
@@ -44,19 +48,13 @@ AccessControlDialog::AccessControlDialog(QWindow* parent,
     }
     setWindowFlags(windowFlags() | Qt::WindowStaysOnTopHint);
 
-    m_ui->setupUi(this);
-
-    connect(m_ui->cancelButton, &QPushButton::clicked, this, [this]() { done(DenyAll); });
-    connect(m_ui->allowButton, &QPushButton::clicked, this, [this]() { done(AllowSelected); });
-    connect(m_ui->itemsTable, &QTableView::clicked, m_model.data(), &EntryModel::toggleCheckState);
-    connect(m_ui->rememberCheck, &QCheckBox::clicked, this, &AccessControlDialog::rememberChecked);
     connect(this, &QDialog::finished, this, &AccessControlDialog::dialogFinished);
 
-    m_ui->rememberMsg->setCloseButtonVisible(false);
+    m_ui->setupUi(this);
-    m_ui->rememberMsg->setMessageType(MessageWidget::Information);
-
     m_ui->appLabel->setText(m_ui->appLabel->text().arg(app));
 
+    // items table
+    connect(m_ui->itemsTable, &QTableView::clicked, m_model.data(), &EntryModel::toggleCheckState);
     m_ui->itemsTable->setModel(m_model.data());
     installWidgetItemDelegate<DenyButton>(m_ui->itemsTable, 2, [this](QWidget* p, const QModelIndex& idx) {
         auto btn = new DenyButton(p, idx);
@@ -68,19 +66,85 @@ AccessControlDialog::AccessControlDialog(QWindow* parent,
     m_ui->itemsTable->horizontalHeader()->setSectionResizeMode(2, QHeaderView::ResizeToContents);
     m_ui->itemsTable->resizeColumnsToContents();
 
+    // the info widget
+    m_ui->rememberMsg->setMessageType(MessageWidget::Information);
+    m_ui->rememberMsg->show(); // sync with m_rememberCheck->setChecked(true)
+    m_ui->exePathWarn->setMessageType(MessageWidget::Warning);
+    m_ui->exePathWarn->hide();
+
+    setupDetails(info);
+
+    // the button box
+    QString detailsButtonText = tr("Details");
+    auto detailsButton =
+        m_ui->buttonBox->addButton(detailsButtonText + QStringLiteral(" >>"), QDialogButtonBox::HelpRole);
+    detailsButton->setCheckable(true);
+
+    m_rememberCheck = new QCheckBox(tr("Remember"), this);
+    m_rememberCheck->setObjectName("rememberCheck"); // for testing
+    m_rememberCheck->setChecked(true);
+    m_ui->buttonBox->addButton(m_rememberCheck, QDialogButtonBox::ActionRole);
+
+    auto allowButton = m_ui->buttonBox->addButton(tr("Allow Selected"), QDialogButtonBox::AcceptRole);
+    allowButton->setDefault(true);
+
+    auto cancelButton = m_ui->buttonBox->addButton(tr("Deny All"), QDialogButtonBox::RejectRole);
+
+    connect(cancelButton, &QPushButton::clicked, this, [this]() { done(DenyAll); });
+    connect(allowButton, &QPushButton::clicked, this, [this]() { done(AllowSelected); });
+    connect(m_rememberCheck, &QCheckBox::clicked, this, &AccessControlDialog::rememberChecked);
+    connect(detailsButton, &QPushButton::clicked, this, [=](bool checked) {
+        m_ui->detailsContainer->setVisible(checked);
+        if (checked) {
+            detailsButton->setText(detailsButtonText + QStringLiteral(" <<"));
+        } else {
+            detailsButton->setText(detailsButtonText + QStringLiteral(" >>"));
+        }
+        adjustSize();
+    });
+
+    // tune the UI according to options
     if (!authOptions.testFlag(AuthOption::Remember)) {
-        m_ui->rememberCheck->setHidden(true);
+        m_rememberCheck->hide();
-        m_ui->rememberCheck->setChecked(false);
+        m_rememberCheck->setChecked(false);
     }
     if (!authOptions.testFlag(AuthOption::PerEntryDeny)) {
         m_ui->itemsTable->horizontalHeader()->setSectionHidden(2, true);
     }
 
-    m_ui->allowButton->setFocus();
+    // show warning and details if not valid
+    if (!info.valid) {
+        m_ui->exePathWarn->show();
+        detailsButton->click();
+    }
+
+    allowButton->setFocus();
 }
 
 AccessControlDialog::~AccessControlDialog() = default;
 
+void AccessControlDialog::setupDetails(const FdoSecrets::PeerInfo& info)
+{
+    QTreeWidgetItem* item = nullptr;
+    for (auto it = info.hierarchy.crbegin(); it != info.hierarchy.crend(); ++it) {
+        QStringList columns = {
+            it->name,
+            QString::number(it->pid),
+            it->exePath,
+            it->command,
+        };
+        if (item) {
+            item = new QTreeWidgetItem(item, columns);
+        } else {
+            item = new QTreeWidgetItem(m_ui->procTree, columns);
+        }
+    }
+    m_ui->procTree->expandAll();
+    m_ui->procTree->header()->setSectionResizeMode(QHeaderView::ResizeToContents);
+    m_ui->procTree->scrollToBottom();
+    m_ui->detailsContainer->hide();
+}
+
 void AccessControlDialog::rememberChecked(bool checked)
 {
     if (checked) {
@@ -101,8 +165,8 @@ void AccessControlDialog::denyEntryClicked(Entry* entry, const QModelIndex& inde
 
 void AccessControlDialog::dialogFinished(int result)
 {
-    auto allow = m_ui->rememberCheck->isChecked() ? AuthDecision::Allowed : AuthDecision::AllowedOnce;
+    auto allow = m_rememberCheck->isChecked() ? AuthDecision::Allowed : AuthDecision::AllowedOnce;
-    auto deny = m_ui->rememberCheck->isChecked() ? AuthDecision::Denied : AuthDecision::DeniedOnce;
+    auto deny = m_rememberCheck->isChecked() ? AuthDecision::Denied : AuthDecision::DeniedOnce;
 
     for (int row = 0; row != m_model->rowCount({}); ++row) {
         auto entry = m_model->data(m_model->index(row, 2), Qt::EditRole).value<Entry*>();

src/fdosecrets/widgets/AccessControlDialog.h

@@ -21,6 +21,7 @@
 #define KEEPASSXC_FDOSECRETS_ACCESSCONTROLDIALOG_H
 
 #include <QAbstractTableModel>
+#include <QCheckBox>
 #include <QDialog>
 #include <QPointer>
 #include <QPushButton>
@@ -35,6 +36,11 @@ namespace Ui
     class AccessControlDialog;
 }
 
+namespace FdoSecrets
+{
+    struct PeerInfo;
+}
+
 enum class AuthOption
 {
     None = 0,
@@ -52,6 +58,7 @@ public:
     explicit AccessControlDialog(QWindow* parent,
                                  const QList<Entry*>& entries,
                                  const QString& app,
+                                 const FdoSecrets::PeerInfo& info,
                                  AuthOptions authOptions = AuthOption::Remember | AuthOption::PerEntryDeny);
     ~AccessControlDialog() override;
 
@@ -76,7 +83,10 @@ private:
     class EntryModel;
     class DenyButton;
 
+    void setupDetails(const FdoSecrets::PeerInfo& info);
+
     QScopedPointer<Ui::AccessControlDialog> m_ui;
+    QPointer<QCheckBox> m_rememberCheck;
     QScopedPointer<EntryModel> m_model;
     QHash<Entry*, AuthDecision> m_decisions;
 };

src/fdosecrets/widgets/AccessControlDialog.ui

@@ -14,6 +14,16 @@
    <string>KeePassXC -  Access Request</string>
   </property>
   <layout class="QVBoxLayout" name="verticalLayout">
+   <item>
+    <widget class="MessageWidget" name="exePathWarn" native="true">
+     <property name="text" stdset="0">
+      <string>Non-existing/inaccessible executable path. Please double-check the client is legit.</string>
+     </property>
+     <property name="closeButtonVisible" stdset="0">
+      <bool>false</bool>
+     </property>
+    </widget>
+   </item>
    <item>
     <widget class="QLabel" name="appLabel">
      <property name="font">
@@ -30,8 +40,62 @@
      </property>
     </widget>
    </item>
+   <item>
+    <widget class="QWidget" name="detailsContainer" native="true">
+     <layout class="QVBoxLayout" name="verticalLayout_2">
+      <item>
+       <widget class="QTreeWidget" name="procTree">
+        <property name="sizePolicy">
+         <sizepolicy hsizetype="Expanding" vsizetype="Preferred">
+          <horstretch>0</horstretch>
+          <verstretch>0</verstretch>
+         </sizepolicy>
+        </property>
+        <property name="uniformRowHeights">
+         <bool>true</bool>
+        </property>
+        <property name="animated">
+         <bool>true</bool>
+        </property>
+        <property name="allColumnsShowFocus">
+         <bool>true</bool>
+        </property>
+        <attribute name="headerStretchLastSection">
+         <bool>false</bool>
+        </attribute>
+        <column>
+         <property name="text">
+          <string>Name</string>
+         </property>
+        </column>
+        <column>
+         <property name="text">
+          <string>PID</string>
+         </property>
+        </column>
+        <column>
+         <property name="text">
+          <string>Executable</string>
+         </property>
+        </column>
+        <column>
+         <property name="text">
+          <string>Command Line</string>
+         </property>
+        </column>
+       </widget>
+      </item>
+     </layout>
+    </widget>
+   </item>
    <item>
     <widget class="QTableView" name="itemsTable">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Expanding" vsizetype="Preferred">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
      <property name="editTriggers">
       <set>QAbstractItemView::NoEditTriggers</set>
      </property>
@@ -63,56 +127,17 @@
      <property name="text" stdset="0">
       <string>Your decision for above entries will be remembered for the duration the requesting client is running.</string>
      </property>
+     <property name="closeButtonVisible" stdset="0">
+      <bool>false</bool>
+     </property>
     </widget>
    </item>
    <item>
     <layout class="QHBoxLayout" name="horizontalLayout">
      <item>
-      <spacer name="horizontalSpacer">
+      <widget class="QDialogButtonBox" name="buttonBox">
-       <property name="orientation">
+       <property name="standardButtons">
-        <enum>Qt::Horizontal</enum>
+        <set>QDialogButtonBox::NoButton</set>
-       </property>
-       <property name="sizeHint" stdset="0">
-        <size>
-         <width>40</width>
-         <height>20</height>
-        </size>
-       </property>
-      </spacer>
-     </item>
-     <item>
-      <widget class="QCheckBox" name="rememberCheck">
-       <property name="text">
-        <string>Remember</string>
-       </property>
-       <property name="checked">
-        <bool>true</bool>
-       </property>
-      </widget>
-     </item>
-     <item>
-      <widget class="QPushButton" name="allowButton">
-       <property name="accessibleName">
-        <string>Allow access to entries</string>
-       </property>
-       <property name="text">
-        <string>Allow Selected</string>
-       </property>
-       <property name="autoDefault">
-        <bool>true</bool>
-       </property>
-       <property name="default">
-        <bool>true</bool>
-       </property>
-      </widget>
-     </item>
-     <item>
-      <widget class="QPushButton" name="cancelButton">
-       <property name="text">
-        <string>Deny All</string>
-       </property>
-       <property name="autoDefault">
-        <bool>true</bool>
        </property>
       </widget>
      </item>

src/fdosecrets/widgets/SettingsModels.cpp

@@ -28,6 +28,8 @@
 
 namespace FdoSecrets
 {
+    // static constexpr still requires definition before c++17
+    constexpr const char* SettingsDatabaseModel::ColumnNames[];
 
     SettingsDatabaseModel::SettingsDatabaseModel(DatabaseTabWidget* dbTabs, QObject* parent)
         : QAbstractTableModel(parent)
@@ -58,7 +60,7 @@ namespace FdoSecrets
         if (parent.isValid()) {
             return 0;
         }
-        return 3;
+        return sizeof(ColumnNames) / sizeof(ColumnNames[0]);
     }
 
     QVariant SettingsDatabaseModel::headerData(int section, Qt::Orientation orientation, int role) const
@@ -71,16 +73,11 @@ namespace FdoSecrets
             return {};
         }
 
-        switch (section) {
+        if (section < 0 || section >= columnCount({})) {
-        case 0:
-            return tr("File Name");
-        case 1:
-            return tr("Group");
-        case 2:
-            return tr("Manage");
-        default:
             return {};
         }
+
+        return qApp->translate(metaObject()->className(), ColumnNames[section]);
     }
 
     QVariant SettingsDatabaseModel::data(const QModelIndex& index, int role) const
@@ -88,17 +85,24 @@ namespace FdoSecrets
         if (!index.isValid()) {
             return {};
         }
+        if (index.model() != this) {
+            return {};
+        }
+        if (index.row() >= rowCount({}) || index.column() >= columnCount({})) {
+            return {};
+        }
+
         const auto& dbWidget = m_dbs[index.row()];
         if (!dbWidget) {
             return {};
         }
 
         switch (index.column()) {
-        case 0:
+        case ColumnFileName:
             return dataForName(dbWidget, role);
-        case 1:
+        case ColumnGroup:
             return dataForExposedGroup(dbWidget, role);
-        case 2:
+        case ColumnManage:
             return dataForManage(dbWidget, role);
         default:
             return {};
@@ -240,6 +244,9 @@ namespace FdoSecrets
         }
     }
 
+    // static constexpr still requires definition before c++17
+    constexpr const char* SettingsClientModel::ColumnNames[];
+
     SettingsClientModel::SettingsClientModel(DBusMgr& dbus, QObject* parent)
         : QAbstractTableModel(parent)
         , m_dbus(dbus)
@@ -260,7 +267,7 @@ namespace FdoSecrets
         if (parent.isValid()) {
             return 0;
         }
-        return 2;
+        return sizeof(ColumnNames) / sizeof(ColumnNames[0]);
     }
 
     QVariant SettingsClientModel::headerData(int section, Qt::Orientation orientation, int role) const
@@ -273,14 +280,11 @@ namespace FdoSecrets
             return {};
         }
 
-        switch (section) {
+        if (section < 0 || section >= columnCount({})) {
-        case 0:
-            return tr("Application");
-        case 1:
-            return tr("Manage");
-        default:
             return {};
         }
+
+        return qApp->translate(metaObject()->className(), ColumnNames[section]);
     }
 
     QVariant SettingsClientModel::data(const QModelIndex& index, int role) const
@@ -288,15 +292,26 @@ namespace FdoSecrets
         if (!index.isValid()) {
             return {};
         }
+        if (index.model() != this) {
+            return {};
+        }
+        if (index.row() >= rowCount({}) || index.column() >= columnCount({})) {
+            return {};
+        }
+
         const auto& client = m_clients[index.row()];
         if (!client) {
             return {};
         }
 
         switch (index.column()) {
-        case 0:
+        case ColumnApplication:
             return dataForApplication(client, role);
-        case 1:
+        case ColumnPID:
+            return dataForPID(client, role);
+        case ColumnDBus:
+            return dataForDBus(client, role);
+        case ColumnManage:
             return dataForManage(client, role);
         default:
             return {};
@@ -305,9 +320,44 @@ namespace FdoSecrets
 
     QVariant SettingsClientModel::dataForApplication(const DBusClientPtr& client, int role) const
     {
+        const auto& info = client->processInfo();
         switch (role) {
         case Qt::DisplayRole:
-            return client->name();
+            if (info.exePath().isEmpty()) {
+                return tr("Unknown");
+            }
+            return info.exePath();
+        case Qt::ToolTipRole:
+            if (!info.valid) {
+                return tr("Non-existing/inaccessible executable path. Please double-check the client is legit.");
+            }
+            return {};
+        case Qt::DecorationRole:
+            // give some visual clues if the path is invalid
+            if (!info.valid) {
+                return icons()->icon(QStringLiteral("dialog-warning"));
+            }
+            return {};
+        default:
+            return {};
+        }
+    }
+
+    QVariant SettingsClientModel::dataForPID(const DBusClientPtr& client, int role) const
+    {
+        switch (role) {
+        case Qt::DisplayRole:
+            return client->pid();
+        default:
+            return {};
+        }
+    }
+
+    QVariant SettingsClientModel::dataForDBus(const DBusClientPtr& client, int role) const
+    {
+        switch (role) {
+        case Qt::DisplayRole:
+            return client->address();
         default:
             return {};
         }

src/fdosecrets/widgets/SettingsModels.h

@@ -40,6 +40,18 @@ namespace FdoSecrets
         QVariant data(const QModelIndex& index, int role) const override;
         QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
 
+        enum Column
+        {
+            ColumnFileName,
+            ColumnGroup,
+            ColumnManage,
+        };
+        static constexpr const char* ColumnNames[] = {
+            QT_TRANSLATE_NOOP("SettingsDatabaseModel", "File Name"),
+            QT_TRANSLATE_NOOP("SettingsDatabaseModel", "Group"),
+            QT_TRANSLATE_NOOP("SettingsDatabaseModel", "Manage"),
+        };
+
     private:
         QVariant dataForName(DatabaseWidget* db, int role) const;
         static QVariant dataForExposedGroup(DatabaseWidget* db, int role);
@@ -71,8 +83,24 @@ namespace FdoSecrets
         QVariant data(const QModelIndex& index, int role) const override;
         QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
 
+        enum Column
+        {
+            ColumnApplication,
+            ColumnPID,
+            ColumnDBus,
+            ColumnManage,
+        };
+        static constexpr const char* ColumnNames[] = {
+            QT_TRANSLATE_NOOP("SettingsClientModel", "Application"),
+            QT_TRANSLATE_NOOP("SettingsClientModel", "PID"),
+            QT_TRANSLATE_NOOP("SettingsClientModel", "DBus Address"),
+            QT_TRANSLATE_NOOP("SettingsClientModel", "Manage"),
+        };
+
     private:
         QVariant dataForApplication(const DBusClientPtr& client, int role) const;
+        QVariant dataForPID(const DBusClientPtr& client, int role) const;
+        QVariant dataForDBus(const DBusClientPtr& client, int role) const;
         QVariant dataForManage(const DBusClientPtr& client, int role) const;
 
     private slots:

src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp

@@ -209,28 +209,30 @@ SettingsWidgetFdoSecrets::SettingsWidgetFdoSecrets(FdoSecretsPlugin* plugin, QWi
 
     auto clientModel = new SettingsClientModel(*plugin->dbus(), this);
     m_ui->tableClients->setModel(clientModel);
-    installWidgetItemDelegate<ManageSession>(
+    installWidgetItemDelegate<ManageSession>(m_ui->tableClients,
-        m_ui->tableClients, 1, [](QWidget* p, const QModelIndex&) { return new ManageSession(p); });
+                                             SettingsClientModel::ColumnManage,
+                                             [](QWidget* p, const QModelIndex&) { return new ManageSession(p); });
 
     // config header after setting model, otherwise the header doesn't have enough sections
     auto clientViewHeader = m_ui->tableClients->horizontalHeader();
     clientViewHeader->setSelectionMode(QAbstractItemView::NoSelection);
     clientViewHeader->setSectionsClickable(false);
-    clientViewHeader->setSectionResizeMode(0, QHeaderView::Stretch); // application
+    clientViewHeader->setSectionResizeMode(QHeaderView::ResizeToContents);
-    clientViewHeader->setSectionResizeMode(1, QHeaderView::ResizeToContents); // disconnect button
+    clientViewHeader->setSectionResizeMode(SettingsClientModel::ColumnApplication, QHeaderView::Stretch);
 
     auto dbModel = new SettingsDatabaseModel(plugin->dbTabs(), this);
     m_ui->tableDatabases->setModel(dbModel);
     installWidgetItemDelegate<ManageDatabase>(
-        m_ui->tableDatabases, 2, [plugin](QWidget* p, const QModelIndex&) { return new ManageDatabase(plugin, p); });
+        m_ui->tableDatabases, SettingsDatabaseModel::ColumnManage, [plugin](QWidget* p, const QModelIndex&) {
+            return new ManageDatabase(plugin, p);
+        });
 
     // config header after setting model, otherwise the header doesn't have enough sections
     auto dbViewHeader = m_ui->tableDatabases->horizontalHeader();
     dbViewHeader->setSelectionMode(QAbstractItemView::NoSelection);
     dbViewHeader->setSectionsClickable(false);
-    dbViewHeader->setSectionResizeMode(0, QHeaderView::Stretch); // file name
+    dbViewHeader->setSectionResizeMode(QHeaderView::Stretch);
-    dbViewHeader->setSectionResizeMode(1, QHeaderView::Stretch); // group
+    dbViewHeader->setSectionResizeMode(SettingsDatabaseModel::ColumnManage, QHeaderView::ResizeToContents);
-    dbViewHeader->setSectionResizeMode(2, QHeaderView::ResizeToContents); // manage button
 
     // prompt the user to save settings before the sections are enabled
     connect(m_plugin, &FdoSecretsPlugin::secretServiceStarted, this, &SettingsWidgetFdoSecrets::updateServiceState);

tests/gui/TestGuiFdoSecrets.cpp

@@ -117,7 +117,9 @@ class FakeClient : public DBusClient
 {
 public:
     explicit FakeClient(DBusMgr* dbus)
-        : DBusClient(dbus, QStringLiteral("local"), 0, "fake-client")
+        : DBusClient(
+            dbus,
+            {QStringLiteral("local"), 0, true, {ProcInfo{0, 0, QStringLiteral("fake-client"), QString{}, QString{}}}})
     {
     }
 };