Move the security claims into the compiler and CI

.github/workflows/audit.yml

@@ -21,6 +21,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
       - uses: EmbarkStudios/cargo-deny-action@3c6349835b2b7b196a839186cb8b78e02f7b5f25 # v2.1.1
         with:
           command: check
@@ -33,6 +35,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
+        with:
+          persist-credentials: false
       - uses: rustsec/audit-check@858dc40f52ca2b8570b7a997c1c4e35c6fc9a432 # node24 (master, "Update to use Node 24" #48; v2.0.0 was the GitHub-deprecated node20)
         with:
           token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/ci.yml

@@ -19,6 +19,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable
         with:
           toolchain: stable
@@ -34,6 +36,8 @@ jobs:
         os: [ubuntu-latest, macos-latest, windows-latest]
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8
         with:
           toolchain: stable
@@ -58,6 +62,8 @@ jobs:
         os: [ubuntu-latest, macos-latest, windows-latest]
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8
         with:
           toolchain: stable
@@ -74,6 +80,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8
         with:
           toolchain: "1.88"
@@ -85,6 +93,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8
         with:
           toolchain: stable
@@ -103,6 +113,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
       - name: the crate and the extension must be the same version
         run: |
           crate=$(sed -n 's/^version = "\(.*\)"/\1/p' Cargo.toml | head -1)
@@ -123,6 +135,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
       - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
         with:
           node-version: 24
@@ -131,6 +145,42 @@ jobs:
       - run: npm --prefix extension ci
       - run: npm --prefix extension run typecheck
       - run: npm --prefix extension run build
+      # What the extension can touch, against what is written down. Every key it compares
+      # decides something a reviewer would ask about -- which APIs, which origins, which
+      # pages -- and those are the ones that change by accident, because a key added while
+      # doing something else looks like nothing in a diff full of TypeScript.
+      - run: node extension/check-permissions.mjs
+      # The build dependencies are esbuild and typescript, and they run on this machine with
+      # the repository checked out. A compromised one is a compromised build, so the advisory
+      # database is worth a second here. `high` and above only: the moderate tier is mostly
+      # transitive noise in dev tooling, and a check that cries wolf gets disabled.
+      - run: npm --prefix extension audit --audit-level=high
+
+  workflows:
+    # The checks above are only worth what the runner they execute on is worth. zizmor reads
+    # these files for the ways a workflow leaks its own privileges: a `${{ }}` interpolated
+    # into a `run:` block where a branch name can reach it, a token wider than the job needs,
+    # an action referenced by a tag that can be moved under it.
+    #
+    # Advisory-only would make it decoration. It fails the build, and an accepted finding is
+    # a `# zizmor: ignore[rule]` comment with a reason beside it.
+    #
+    # It is also why every `actions/checkout` in this repository carries
+    # `persist-credentials: false`: the default leaves a token in `.git/config`, which every
+    # later step can read and any artifact upload can carry off the runner. The three that
+    # need git credentials say so where they are.
+    name: workflows
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
+      # `pipx` is on the runner image, so this needs no action and therefore no fourth-party
+      # SHA to keep pinned. The version is exact: a linter that moves under you turns a green
+      # branch red for reasons nobody changed.
+      - run: pipx run zizmor==1.30.1 --persona=regular .github/workflows/
 
   e2e:
     # The only check that tests the claim rather than a piece of it: that a file on a host
@@ -142,6 +192,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8
         with:
           toolchain: stable
@@ -192,6 +244,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8
         with:
           toolchain: stable

.github/workflows/codeql.yml

@@ -40,6 +40,8 @@ jobs:
         language: [rust]
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
 
       - name: Initialize CodeQL
         uses: github/codeql-action/init@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0

.github/workflows/coverage.yml

@@ -25,6 +25,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8  # stable
         with:
           components: llvm-tools-preview

.github/workflows/dco.yml

@@ -35,6 +35,7 @@ jobs:
       - name: Checkout PR head (full history so the base merge-base resolves)
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
         with:
+          persist-credentials: false
           fetch-depth: 0
 
       - name: Fetch the base branch

.github/workflows/msrv-drift.yml

@@ -34,6 +34,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8
         with:
           toolchain: "1.88"

.github/workflows/release-plz.yml

@@ -78,7 +78,10 @@ jobs:
     if: needs.release.outputs.released == 'true'
     permissions:
       contents: write
-    uses: ./.github/workflows/release-assets.yml
+    # `$/` is the better spelling, and this is the one workflow whose first real run is a
+    # release. A path that resolves today is worth more here than a path that reads better;
+    # revisit once a release has gone out on it.
+    uses: ./.github/workflows/release-assets.yml # zizmor: ignore[self-repository]
     with:
       tag: ${{ needs.release.outputs.tag }}
 

.github/workflows/typos.yml

@@ -19,4 +19,6 @@ jobs:
     timeout-minutes: 5
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1   # v7.0.1
+        with:
+          persist-credentials: false
       - uses: crate-ci/typos@d43b6c087ac471e2ea7b8af622ff15f05c0c365b     # v1.50.1

Cargo.toml

@@ -8,3 +8,9 @@ edition = "2024"
 rust-version = "1.88"
 license = "MIT"
 repository = "https://github.com/QAtlasHub/ssh-browser"
+
+[workspace.lints.rust]
+# There is none today. Forbidding it means there is none tomorrow either, without anyone
+# having to look: `forbid` cannot be lifted by an `allow` further down, so adding the first
+# `unsafe` block is a build failure rather than a line in a diff nobody reads.
+unsafe_code = "forbid"

crates/ssh-browser/Cargo.toml

@@ -12,6 +12,9 @@ keywords = ["ssh", "sftp", "browser", "proxy", "origin"]
 # from a shell would be neither.
 categories = ["network-programming", "command-line-utilities"]
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow = "1"
 bytes = "1"

crates/ssh-browser/src/fs/sftp.rs

@@ -19,7 +19,7 @@ use super::{Entry, RangeReq, Refused, RemoteFs};
 use crate::sftp::transport::{self, SshChild};
 use crate::sftp::wire::{
     Attrs, CLOSE, DATA, Dec, Enc, FXF_READ, HANDLE, NAME, OPEN, OPENDIR, READ, READDIR, REALPATH,
-    STATUS, STATUS_EOF,
+    STATUS, STATUS_EOF, Verb,
 };
 use crate::sftp::{Reply, Rx, Sftp, Tx};
 
@@ -34,7 +34,7 @@ const READ_CHUNK: u32 = 32 * 1024;
 const COALESCE: Duration = Duration::from_micros(200);
 
 struct Job {
-    kind: u8,
+    kind: Verb,
     /// Request body without the leading id; the writer owns id allocation.
     body: Vec<u8>,
     reply: oneshot::Sender<Reply>,
@@ -93,7 +93,7 @@ impl SftpFs {
     }
 
     /// Hand a request to the writer without awaiting its reply.
-    async fn issue(&self, kind: u8, body: Vec<u8>) -> Result<oneshot::Receiver<Reply>> {
+    async fn issue(&self, kind: Verb, body: Vec<u8>) -> Result<oneshot::Receiver<Reply>> {
         let (reply, rx) = oneshot::channel();
         self.jobs
             .send(Job { kind, body, reply })
@@ -526,6 +526,15 @@ async fn reader<R: AsyncRead + Unpin>(mut rx: Rx<R>, pending: Pending) {
 mod tests {
     use super::*;
     use crate::sftp::wire::{INIT, VERSION};
+
+    /// The bytes a request arrives as, for the fake server below to match on.
+    ///
+    /// Spelled out here rather than matched on `Verb` directly, because what comes off a socket is
+    /// a byte and a byte is not a request this daemon may send -- which is the whole distinction
+    /// `Verb` exists to keep. Derived from the constants, so they cannot drift apart.
+    const B_OPEN: u8 = OPEN.code();
+    const B_CLOSE: u8 = CLOSE.code();
+    const B_READ: u8 = READ.code();
     use crate::sftp::{read_frame, write_frame};
     use tokio::io::AsyncWriteExt;
 
@@ -547,8 +556,8 @@ mod tests {
             let mut d = Dec::new(&payload);
             let id = d.u32().expect("request id");
             let (out_kind, out) = match kind {
-                OPEN => (HANDLE, Enc::new().u32(id).str(b"h").done()),
-                READ => {
+                B_OPEN => (HANDLE, Enc::new().u32(id).str(b"h").done()),
+                B_READ => {
                     d.str().expect("handle");
                     let offset = d.u64().expect("offset") as usize;
                     if fail_read {
@@ -571,7 +580,7 @@ mod tests {
                         (DATA, Enc::new().u32(id).str(&body[offset..]).done())
                     }
                 }
-                CLOSE => (STATUS, Enc::new().u32(id).u32(0).str(b"ok").str(b"").done()),
+                B_CLOSE => (STATUS, Enc::new().u32(id).u32(0).str(b"ok").str(b"").done()),
                 other => panic!("fake server got unexpected request type {other}"),
             };
             write_frame(&mut w, out_kind, &out).await.expect("reply");

crates/ssh-browser/src/sftp/mod.rs

@@ -15,7 +15,7 @@ pub mod wire;
 use anyhow::{Context, Result, bail, ensure};
 use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
 
-use wire::{Dec, Enc, INIT, VERSION};
+use wire::{Dec, Enc, INIT, VERSION, Verb};
 
 const MAX_FRAME: usize = 64 * 1024 * 1024;
 
@@ -66,8 +66,13 @@ impl<W: AsyncWrite + Unpin> Tx<W> {
         id
     }
 
-    pub async fn queue(&mut self, kind: u8, payload: &[u8]) -> Result<()> {
-        write_frame(&mut self.w, kind, payload).await
+    /// Send one request.
+    ///
+    /// A `Verb`, not a byte. `write_frame` below still takes a byte because a reply is a byte
+    /// and the test server writes replies -- but nothing outside this module reaches it to
+    /// send a *request*, and `Verb` has six values. See `wire::Verb`.
+    pub async fn queue(&mut self, kind: Verb, payload: &[u8]) -> Result<()> {
+        write_frame(&mut self.w, kind.code(), payload).await
     }
 
     pub async fn flush(&mut self) -> Result<()> {
@@ -124,7 +129,7 @@ impl<W: AsyncWrite + Unpin, R: AsyncRead + Unpin> Sftp<W, R> {
         self.tx.alloc_id()
     }
 
-    pub async fn queue(&mut self, kind: u8, payload: &[u8]) -> Result<()> {
+    pub async fn queue(&mut self, kind: Verb, payload: &[u8]) -> Result<()> {
         self.tx.queue(kind, payload).await
     }
 

crates/ssh-browser/src/sftp/wire.rs

@@ -1,14 +1,40 @@
 //! SFTP v3 wire format.
 
+/// A request this daemon can send.
+///
+/// The README says nothing is written to your remote, and this is the sentence that makes it
+/// true: `Verb` wraps a `u8` that nothing outside this module can construct, and there are six
+/// of them. `SSH_FXP_WRITE`, `SETSTAT`, `REMOVE`, `MKDIR`, `RMDIR`, `RENAME` and `SYMLINK` are
+/// not missing by policy -- there is no value of this type that means them, so the code that
+/// would send one does not compile.
+///
+/// A seventh means adding a `pub const` here, which is a line in a diff somebody reads. That
+/// is the point: the claim stops being a thing to remember and becomes a thing to notice.
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+pub struct Verb(u8);
+
+impl Verb {
+    /// The byte that goes on the wire.
+    ///
+    /// One way only. There is no `from_u8`, because a byte that arrived from somewhere is a
+    /// reply type or somebody's input, and neither is a request this may send.
+    pub const fn code(self) -> u8 {
+        self.0
+    }
+}
+
+pub const OPEN: Verb = Verb(3);
+pub const CLOSE: Verb = Verb(4);
+pub const READ: Verb = Verb(5);
+pub const OPENDIR: Verb = Verb(11);
+pub const READDIR: Verb = Verb(12);
+pub const REALPATH: Verb = Verb(16);
+
+/// The version handshake, which is not a filesystem request and so is not a `Verb`.
 pub const INIT: u8 = 1;
 pub const VERSION: u8 = 2;
-pub const OPEN: u8 = 3;
-pub const CLOSE: u8 = 4;
-pub const READ: u8 = 5;
-pub const LSTAT: u8 = 7;
-pub const OPENDIR: u8 = 11;
-pub const READDIR: u8 = 12;
-pub const REALPATH: u8 = 16;
+
+/// Reply types. These arrive; they are never sent, which is why they stay bytes.
 pub const STATUS: u8 = 101;
 pub const HANDLE: u8 = 102;
 pub const DATA: u8 = 103;
@@ -160,6 +186,32 @@ impl Attrs {
 mod tests {
     use super::*;
 
+    /// The README says nothing is written to your remote. This is the list that claim is about.
+    ///
+    /// It fails two ways, and both are the point. Adding a `Verb` and forgetting this list
+    /// fails it; deleting one that is in use fails to compile. So the set cannot grow quietly,
+    /// which is the only way a read-only client stops being one.
+    #[test]
+    fn the_verbs_are_the_six_read_ones() {
+        let mut codes = [OPEN, CLOSE, READ, OPENDIR, READDIR, REALPATH].map(Verb::code);
+        codes.sort_unstable();
+        assert_eq!(codes, [3, 4, 5, 11, 12, 16]);
+
+        // SSH_FXP_WRITE, SETSTAT, FSETSTAT, REMOVE, MKDIR, RMDIR, RENAME, SYMLINK. Named here
+        // so that the absence is written down: a reader should not have to know SFTP v3 by
+        // heart to see that the dangerous half of it is not in the list above.
+        for writes in [6u8, 9, 10, 13, 14, 15, 18, 20] {
+            assert!(!codes.contains(&writes), "{writes} is a write");
+        }
+    }
+
+    /// The only open mode there is. `FXF_WRITE`, `FXF_APPEND`, `FXF_CREAT` and `FXF_TRUNC` are
+    /// not defined anywhere in this crate, so `OPEN` has nothing else it could ask for.
+    #[test]
+    fn the_only_open_flag_is_read() {
+        assert_eq!(FXF_READ, 1);
+    }
+
     #[test]
     fn roundtrips_primitives() {
         let bytes = Enc::new().u32(7).u64(1 << 40).str(b"hi").done();