From 03e902a2d244244d1104b4e6ae8394ddc00ea5e4 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Fri, 31 Jul 2026 12:17:56 +0200 Subject: [PATCH] Colour the host's file rows by what their mode says MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remote pane's NAME column was blue for a directory and plain for everything else, and the PERMS column was faint whatever it said. Two colours now come off the mode, split across those two columns on purpose: NAME says what a row is, so a file with an execute bit is green there, and PERMS says what is notable about how it is set, so a file anyone may write to is amber over the characters that actually say so. Because the two never compete for one TextBlock, a world-writable executable shows both facts instead of one winning an argument. No new blue is spent, which is what App.axaml asks for: it reserves blue for a directory, a distinct scope, and calls it deliberately rare. Both are files only, and each exclusion is a wrong answer avoided rather than a case not got to. Every symbolic link is lrwxrwxrwx by convention and its mode governs nothing — what may be written is the target, whose mode an lstat listing never fetched — so amber there would fire on every link on the host. A world-writable directory is /tmp, made safe by a sticky bit PosixMode does not render, and warning about it would be warning about the half of the mode that is on screen while the half that answers the warning is not. And the execute bit on a directory means "may be searched", which is true of very nearly every directory a host has, so green there would paint the whole pane and mark nothing. The two questions read back the string PosixMode wrote rather than carrying its nine booleans through SftpEntry as well. That is the point rather than a shortcut: two representations of one fact is how a row ends up coloured for a bit the column beside it does not show. A mode of the wrong length answers false rather than throwing, since these decide a colour and a listing is not worth failing over one. The amber is Warn rather than WarnText, which is the muted amber a warning card writes its sentences in. At 9.5px against TextFaint that one is a shade rather than a signal, and a marker nobody notices is the same as no marker. The local pane is untouched, on the grounds it already gives for having no PERMS column at all: a POSIX mode is not a fact about a file on Windows, and colouring one there would invent exactly what the column declines to print. Twenty cases in RemotePathTests, which needs no container — the execute bit in any of the three triples rather than only the owner's, the others-write bit alone, a mode of the wrong length, and the file-only rule for both questions from all three kinds. dotnet format is clean and the app and layout suites pass at 109 and 35. --- .../ViewModels/TransfersViewModel.cs | 6 ++ .../Views/TransfersScreen.axaml | 34 +++++++++- src/DodoSSH.Client.Ssh/SftpSession.cs | 47 +++++++++++++ .../RemotePathTests.cs | 67 +++++++++++++++++++ 4 files changed, 152 insertions(+), 2 deletions(-) diff --git a/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs b/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs index 6be72ec..a275d13 100644 --- a/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs @@ -37,6 +37,12 @@ internal sealed class RemoteEntryRowViewModel(SftpEntry entry) /// The mode as drwxr-xr-x, which is the design's PERMS column. internal string Permissions => entry.Permissions; + + /// Whether the row is a file with an execute bit, which the NAME column colours for. + internal bool IsExecutable => entry.IsExecutable; + + /// Whether the row is a file anyone may write to, which the PERMS column colours for. + internal bool IsWorldWritable => entry.IsWorldWritable; } /// One local file or directory, as a row. diff --git a/src/DodoSSH.Client.App/Views/TransfersScreen.axaml b/src/DodoSSH.Client.App/Views/TransfersScreen.axaml index 72cd513..6408529 100644 --- a/src/DodoSSH.Client.App/Views/TransfersScreen.axaml +++ b/src/DodoSSH.Client.App/Views/TransfersScreen.axaml @@ -23,6 +23,16 @@ A directory is marked by colour rather than by an icon: this application ships no icon set, and the palette already reserves blue for "a directory, a distinct scope" — see App.axaml, where it is described as deliberately rare. This is the one place it is spent. + + Two further colours come from the mode, and they are split across the two columns on purpose: NAME says + what a row is, PERMS says what is notable about how it is set. So an executable is green in NAME — + "live, yours, something that runs" — while a file anyone may write to is amber in PERMS, over the + characters that actually say so. The two never compete for one TextBlock, which is what lets a + world-writable executable show both facts instead of one winning an argument. + + Both are files only; see SftpEntry, which will not read a mode off a symbolic link or a directory. + Rendering `-rwxrwxrwx` in two colours at once is not something this list can do, so amber over the whole + string is the compromise: the eye lands on the column, and the string itself is the detail. --> + + + + + + @@ -291,14 +320,15 @@ - + diff --git a/src/DodoSSH.Client.Ssh/SftpSession.cs b/src/DodoSSH.Client.Ssh/SftpSession.cs index 6bff153..9f802cc 100644 --- a/src/DodoSSH.Client.Ssh/SftpSession.cs +++ b/src/DodoSSH.Client.Ssh/SftpSession.cs @@ -51,6 +51,32 @@ public sealed record SftpEntry( /// file fails the listing instead, which is the caller's cue that it was not a directory after all. /// public bool IsNavigable => Kind is SftpEntryKind.Directory or SftpEntryKind.SymbolicLink; + + /// Whether this is a file somebody can run. + /// + /// Files only. On a directory the execute bit means "may be searched", which is true of very nearly every + /// directory on a host — a listing that marked them all would be marking nothing. + /// + public bool IsExecutable => Kind is SftpEntryKind.File && PosixMode.HasAnyExecuteBit(Permissions); + + /// + /// Whether this is a file any account on the host may write to. + /// + /// + /// + /// Files only, and for two separate reasons. A symbolic link is lrwxrwxrwx by convention on every + /// system that has one, and its mode governs nothing: what may be written is the target, whose own mode + /// this listing did not fetch. And a directory that everyone may write to is the ordinary arrangement for + /// /tmp, made safe by the sticky bit — which does not render, so flagging + /// the directory would be warning about the half of the mode that is on screen while the half that + /// answers the warning is not. + /// + /// + /// It is not a claim that writing is dangerous, only that the mode says something a reader of that column + /// would want to have noticed. + /// + /// + public bool IsWorldWritable => Kind is SftpEntryKind.File && PosixMode.IsWorldWritable(Permissions); } /// @@ -106,6 +132,27 @@ public static class PosixMode triple[2] = execute ? 'x' : '-'; } } + + /// Whether any of the three execute bits is set. + public static bool HasAnyExecuteBit(string mode) => + At(mode, OwnerExecute) == 'x' || At(mode, GroupExecute) == 'x' || At(mode, OthersExecute) == 'x'; + + /// Whether the others triple carries the write bit. + public static bool IsWorldWritable(string mode) => At(mode, OthersWrite) == 'w'; + + private const int OwnerExecute = 3; + private const int GroupExecute = 6; + private const int OthersWrite = 8; + private const int OthersExecute = 9; + + /// + /// Reading back what wrote, rather than carrying the nine booleans through + /// as well. The alternative is a second representation of one fact, and the two + /// disagreeing is the failure this avoids — a row coloured for a bit the column beside it does not show. + /// Anything that is not a mode this type wrote answers false rather than throwing: these questions decide + /// a colour, and a listing is not worth failing over one. + /// + private static char At(string mode, int index) => mode.Length == 10 ? mode[index] : '-'; } /// diff --git a/tests/DodoSSH.Client.Ssh.Tests/RemotePathTests.cs b/tests/DodoSSH.Client.Ssh.Tests/RemotePathTests.cs index bed0101..2035349 100644 --- a/tests/DodoSSH.Client.Ssh.Tests/RemotePathTests.cs +++ b/tests/DodoSSH.Client.Ssh.Tests/RemotePathTests.cs @@ -83,6 +83,69 @@ public sealed class RemotePathTests .ShouldBe("lrwxrwxrwx"); } + [Theory] + [InlineData("-rwxr-xr-x", true)] + [InlineData("-rw-r-xr--", true)] + [InlineData("-rw-r--r-x", true)] + [InlineData("-rw-r--r--", false)] + [InlineData("----------", false)] + public void PosixMode_ReadsAnExecuteBitInAnyTriple(string mode, bool expected) + { + // Any of the three, not the owner's: the account browsing is not necessarily the owner, and a file + // that only the group may run is still a file somebody runs. + PosixMode.HasAnyExecuteBit(mode).ShouldBe(expected); + } + + [Theory] + [InlineData("-rw-rw-rw-", true)] + [InlineData("-rw-rw-r--", false)] + [InlineData("-rw-r--r--", false)] + [InlineData("--------w-", true)] + public void PosixMode_ReadsTheOthersWriteBit(string mode, bool expected) + { + // The others triple only. A group-writable file is writable by a named set of people, which is what + // groups are for; this asks about the column that names nobody. + PosixMode.IsWorldWritable(mode).ShouldBe(expected); + } + + [Theory] + [InlineData("")] + [InlineData("rwxrwxrwx")] + [InlineData("?")] + public void PosixMode_AnswersFalseForAModeItDidNotWrite(string mode) + { + // These two decide a row's colour and nothing else. A listing is not worth failing over a string of + // the wrong length, and an index off the end of one is how that would happen. + PosixMode.HasAnyExecuteBit(mode).ShouldBeFalse(); + PosixMode.IsWorldWritable(mode).ShouldBeFalse(); + } + + [Theory] + [InlineData(SftpEntryKind.File, "-rwxr-xr-x", true)] + [InlineData(SftpEntryKind.File, "-rw-r--r--", false)] + [InlineData(SftpEntryKind.Directory, "drwxr-xr-x", false)] + [InlineData(SftpEntryKind.SymbolicLink, "lrwxrwxrwx", false)] + public void AnEntry_IsExecutableOnlyWhenItIsAFile(SftpEntryKind kind, string mode, bool expected) + { + // The x on a directory means "may be searched", which is true of very nearly every directory a host + // has. A file browser that marked them all would be marking nothing. + Entry(kind, mode).IsExecutable.ShouldBe(expected); + } + + [Theory] + [InlineData(SftpEntryKind.File, "-rw-rw-rw-", true)] + [InlineData(SftpEntryKind.File, "-rw-r--r--", false)] + [InlineData(SftpEntryKind.SymbolicLink, "lrwxrwxrwx", false)] + [InlineData(SftpEntryKind.Directory, "drwxrwxrwx", false)] + public void AnEntry_IsWorldWritableOnlyWhenItIsAFile(SftpEntryKind kind, string mode, bool expected) + { + // The two exclusions are the point. Every symbolic link is lrwxrwxrwx and its mode governs nothing — + // what may be written is the target, whose mode this listing never fetched. And a world-writable + // directory is /tmp, made safe by a sticky bit PosixMode does not render: warning there would be + // warning about the half of the mode that is on screen. + Entry(kind, mode).IsWorldWritable.ShouldBe(expected); + } + [Theory] [InlineData(0, "0 B")] [InlineData(1023, "1023 B")] @@ -96,4 +159,8 @@ public sealed class RemotePathTests // "512.3 MB" claims a precision the figure does not have by the time it is that large. ByteSize.Format(bytes).ShouldBe(expected); } + + /// An entry that is nothing but a kind and a mode, which is all these two questions read. + private static SftpEntry Entry(SftpEntryKind kind, string mode) => + new("thing", "/tmp/thing", kind, 0, DateTimeOffset.UnixEpoch, mode); }