41221506800

Committed 31 Oct 2026 04:49PM UTC coverage: 92.603% (-0.03%) from 92.63%

Build # 41221506800

Build Type

push

github

Committed by

web-flow

Commit Message

RealPass::Init: take --path from the pair the store check judged (#2097)

* RealPass::Init: take --path from the pair the store check judged

PathValidator::isPathInStore() compares canonical paths; the --path for
pass came from a lexical relativeFilePath(). With the store setting
reached through a link (~/.password-store -> /data/pass) and a folder
spelled through its target (/data/pass/team), the check passed and
--path became ../../data/pass/team, which pass refuses as a sneaky path.

PathValidator::storeRelativePath() returns the place in the store from
the same canonical pair (isPathInStore() is now its has_value()), and
refuseFolderOutsideStore() hands it to the caller, so the check and the
--path cannot disagree.

Failed safe before (pass refused); no caller spells a folder that way.
Raised by CodeRabbit and Henk on #2096, tracked in #2094.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJygpMUf461yZgfAqwMJ6d
Signed-off-by: Anne Jan Brouwer <brouwer@annejan.com>

* Init: work in the store's own spelling once the folder is placed

A folder spelled through the target of a linked store root passed the
store check (canonical) but was then used as given, and the recipient
list lookups compare against the store as named:

- RealPass walked the tree for its signed-list check in that spelling,
  so each entry's nearest list was not found and deeper rolled-back
  lists went unchecked while pass init (now given a valid --path)
  re-encrypted under them.
- ImitatePass re-encrypted in that spelling, so each entry's nearest
  list fell back to the store root's and entries were encrypted to the
  root's recipients instead of the folder's new list.

Both now continue from the store's spelling of the same folder
(store + its store-relative place). The link checks stay on the given
spelling: a canonical path has no links left to find, and .gpg-id and
.gpg-id.sig are the same files either way.

Found by ... (continued)

Coverage Stats

24 of 26 new or added lines in 4 files covered. (92.31%)

2 existing lines in 2 files now uncovered.

9952 of 10747 relevant lines covered (92.6%)

400.36 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

93.75

/src/pathvalidator.cpp

// SPDX-FileCopyrightText: 2014 Anne Jan Brouwer
// SPDX-License-Identifier: GPL-3.0-or-later

/**
 * @class PathValidator
 * @brief Password-store path-boundary checks implementation.
 *
 * @see pathvalidator.h
 */

#include "pathvalidator.h"
#include <QDir>
#include <QFileInfo>
#include <QStringList>

auto PathValidator::isPathInStore(const QString &storeRoot,
                                  const QString &candidate) -> bool {
  return storeRelativePath(storeRoot, candidate).has_value();
}

auto PathValidator::storeRelativePath(const QString &storeRoot,
                                      const QString &candidate)
    -> std::optional<QString> {
  if (storeRoot.isEmpty() || candidate.isEmpty()) {
    return std::nullopt;
  }
  const QString rootCanon = QDir(storeRoot).canonicalPath();
  if (rootCanon.isEmpty()) {
    return std::nullopt;
  }

  QString resolved;
  const QFileInfo fi(QDir::cleanPath(QFileInfo(candidate).absoluteFilePath()));
  if (fi.exists()) {
    resolved = fi.canonicalFilePath();
  } else {
    // Walk up to the nearest existing ancestor so we can canonicalise it
    // (resolves any symlinks on the way to the store root), then re-append
    // the leaf components below that ancestor.
    QDir parent = fi.dir();
    QStringList tail;
    tail.prepend(fi.fileName());
    while (!parent.exists()) {
      tail.prepend(parent.dirName());
      // The filesystem root always exists, so the loop terminates there
      // before cdUp() can fail; this guard is defensive against a
      // pathological QDir that can neither exist nor ascend.
      if (!parent.cdUp()) {
        return std::nullopt;
      }
    }
    const QString parentCanon = parent.canonicalPath();
    if (parentCanon.isEmpty()) {
      return std::nullopt;
    }
    resolved = QDir::cleanPath(parentCanon + QLatin1Char('/') +
                               tail.join(QLatin1Char('/')));
  }

  if (resolved.isEmpty()) {
    return std::nullopt;
  }
  if (resolved == rootCanon) {
    return QString();
  }
  if (!resolved.startsWith(rootCanon + QLatin1Char('/'))) {
    return std::nullopt;
  }
  return resolved.mid(rootCanon.size() + 1);
}

1	// SPDX-FileCopyrightText: 2014 Anne Jan Brouwer
2	// SPDX-License-Identifier: GPL-3.0-or-later
3
4	/**
5	* @class PathValidator
6	* @brief Password-store path-boundary checks implementation.
7	*
8	* @see pathvalidator.h
9	*/
10
11	#include "pathvalidator.h"
12	#include <QDir>
13	#include <QFileInfo>
14	#include <QStringList>
15
16	auto PathValidator::isPathInStore(const QString &storeRoot,	68 ✔
17	const QString &candidate) -> bool {
18	return storeRelativePath(storeRoot, candidate).has_value();	68 ✔
19	}
20
21	auto PathValidator::storeRelativePath(const QString &storeRoot,	121 ✔
22	const QString &candidate)
23	-> std::optional<QString> {
24	if (storeRoot.isEmpty() \|\| candidate.isEmpty()) {	121 ✔
25	return std::nullopt;	3 ✔
26	}
27	const QString rootCanon = QDir(storeRoot).canonicalPath();	118 ✔
28	if (rootCanon.isEmpty()) {	118 ✔
29	return std::nullopt;	1 ✔
30	}
31
32	QString resolved;	117 ✔
33	const QFileInfo fi(QDir::cleanPath(QFileInfo(candidate).absoluteFilePath()));	234 ✔
34	if (fi.exists()) {	117 ✔
35	resolved = fi.canonicalFilePath();	132 ✔
36	} else {
37	// Walk up to the nearest existing ancestor so we can canonicalise it
38	// (resolves any symlinks on the way to the store root), then re-append
39	// the leaf components below that ancestor.
40	QDir parent = fi.dir();	51 ✔
41	QStringList tail;	51 ✔
42	tail.prepend(fi.fileName());	51 ✔
43	while (!parent.exists()) {	59 ✔
44	tail.prepend(parent.dirName());	8 ✔
45	// The filesystem root always exists, so the loop terminates there
46	// before cdUp() can fail; this guard is defensive against a
47	// pathological QDir that can neither exist nor ascend.
48	if (!parent.cdUp()) {	8 ✔
NEW 49	return std::nullopt;	×
50	}
51	}
52	const QString parentCanon = parent.canonicalPath();	51 ✔
53	if (parentCanon.isEmpty()) {	51 ✔
54	return std::nullopt;
55	}
56	resolved = QDir::cleanPath(parentCanon + QLatin1Char('/') +	102 ✔
57	tail.join(QLatin1Char('/')));	102 ✔
58	}	51 ✔
59
60	if (resolved.isEmpty()) {	117 ✔
NEW 61	return std::nullopt;	×
62	}
63	if (resolved == rootCanon) {	117 ✔
64	return QString();	30 ✔
65	}
66	if (!resolved.startsWith(rootCanon + QLatin1Char('/'))) {	174 ✔
67	return std::nullopt;	22 ✔
68	}
69	return resolved.mid(rootCanon.size() + 1);	130 ✔
70	}	117 ✔

IJHack / QtPass / 41221506800

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous