fix: make attachment path-containment guard separator-aware

Attachment::saveTo()'s escape guard compared paths with a bare
QString::startsWith(), which is not a path-boundary test: "/tmp/safe-evil"
textually starts with "/tmp/safe", so a sibling directory whose name merely
extends the target's name would incorrectly pass as contained within it.

Extract the check into Attachment::isPathInsideDirectory(), comparing
QDir::cleanPath()'d absolute paths and requiring an exact match or a prefix
ending at a '/' boundary. Not exploitable today since safeFilename() always
reduces the name to a bare basename before saveTo() builds the target, so
the guard is unreachable via saveTo()'s public interface; comments on both
now say so plainly instead of implying it is currently load-bearing.

Add pathInsideDirectoryRejectsSiblingPrefix, testing the guard directly
(independent of safeFilename(), which would mask a broken guard by never
producing an escaping path), and safeFilenameStripsPathComponents, testing
the sanitiser that actually stops traversal today.

1 files changed, 20 insertions, 5 deletions

diff --git a/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md b/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md
index b264986..391bcb6 100644
--- a/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md
+++ b/docs/superpowers/plans/2026-08-02-qtmaildir-v1.md
@@ -1000,6 +1000,7 @@ private slots:
     void missingFileIsReported();
     void hostileFilenameIsSanitised();
     void savedAttachmentMatchesBytes();
+    void safeFilenameStripsPathComponents();
 
 private:
     QString fixture(const QString &name) const
@@ -1384,12 +1385,26 @@ QString Attachment::saveTo(const QString &directory, QString *error) const
     const QDir dir(directory);
     const QString target = dir.absoluteFilePath(safeFilename());
 
-    // Belt and braces: confirm the resolved path really is inside directory,
-    // so a future change to safeFilename() cannot silently reintroduce escape.
-    const QString canonicalDir = QDir(directory).absolutePath();
-    if (!QFileInfo(target).absolutePath().startsWith(canonicalDir)) {
+    // Belt and braces, and currently UNREACHABLE through this function:
+    // safeFilename() above already reduces any name to a basename, so no
+    // caller-supplied filename can produce a target outside `directory`.
+    // The guard exists so that a future change which stops sanitising, or
+    // which lets a caller pass a subpath, still cannot escape. Do not write
+    // a test that drives saveTo() expecting a refusal: it cannot happen
+    // while safeFilename() runs first. Test safeFilename() instead, which
+    // is the control that actually stops traversal today.
+    //
+    // The comparison must be separator-aware. A bare startsWith() on the
+    // strings would accept "/tmp/safe-evil/x" as being inside "/tmp/safe",
+    // since one is a string prefix of the other with no path boundary
+    // between them. cleanPath() also resolves ".." before comparison rather
+    // than leaving it to be compared textually.
+    const QString cleanDir = QDir::cleanPath(QDir(directory).absolutePath());
+    const QString cleanTarget = QDir::cleanPath(target);
+    if (cleanTarget != cleanDir
+        && !cleanTarget.startsWith(cleanDir + QLatin1Char('/'))) {
         if (error)
-            *error = QStringLiteral("Refusing to write outside %1").arg(canonicalDir);
+            *error = QStringLiteral("Refusing to write outside %1").arg(cleanDir);
         return {};
     }