backend: add error variant for access denied, handle when diffing

Some backends, like the one we have at Google, can restrict access to
certain files. For such files, if they return a regular
`BackendError::ReadObject`, then that will terminate iteration in many
cases (e.g. when diffing or listing files). This patch adds a new
error variant for them to return instead, plus handling of such errors
in diff output.

In order to test the feature, I added a new commit backend that
returns the new `ReadAccessDenied` error when the caller tries to read
certain objects.

cli/src/config/colors.toml

@@ -81,6 +81,7 @@
 "diff removed" = "red"
 "diff added" = "green"
 "diff modified" = "cyan"
+"diff access-denied" = { bg = "red" }
 
 "op_log id" = "blue"
 "op_log user" = "yellow"

cli/src/diff_util.rs

@@ -38,6 +38,7 @@ use thiserror::Error;
 use tracing::instrument;
 use unicode_width::UnicodeWidthStr as _;
 
+use crate::command_error::{user_error_with_hint, CommandError};
 use crate::config::CommandNameAndArgs;
 use crate::formatter::Formatter;
 use crate::merge_tools::{self, DiffGenerateError, ExternalMergeTool};
@@ -227,7 +228,7 @@ impl<'a> DiffRenderer<'a> {
         from_tree: &MergedTree,
         to_tree: &MergedTree,
         matcher: &dyn Matcher,
-    ) -> Result<(), DiffRenderError> {
+    ) -> Result<(), CommandError> {
         let repo = self.repo;
         let path_converter = self.path_converter;
         for format in &self.formats {
@@ -277,7 +278,7 @@ impl<'a> DiffRenderer<'a> {
         formatter: &mut dyn Formatter,
         commit: &Commit,
         matcher: &dyn Matcher,
-    ) -> Result<(), DiffRenderError> {
+    ) -> Result<(), CommandError> {
         let from_tree = commit.parent_tree(self.repo)?;
         let to_tree = commit.tree()?;
         self.show_diff(ui, formatter, &from_tree, &to_tree, matcher)
@@ -495,7 +496,20 @@ pub fn show_color_words_diff(
     async {
         while let Some((path, diff)) = diff_stream.next().await {
             let ui_path = path_converter.format_file_path(&path);
-            let (left_value, right_value) = diff?;
+            let (left_value, right_value) = match diff {
+                Err(jj_lib::backend::BackendError::ReadAccessDenied {
+                    object_type: _,
+                    hash: _,
+                    source,
+                }) => {
+                    writeln!(
+                        formatter.labeled("access-denied"),
+                        "Access denied to {ui_path}: {source}"
+                    )?;
+                    continue;
+                }
+                other => other?,
+            };
             if left_value.is_absent() {
                 let description = basic_diff_file_type(&right_value);
                 writeln!(
@@ -830,19 +844,29 @@ fn materialized_diff_stream<'a>(
         .buffered((store.concurrency() / 2).max(1))
 }
 
+fn map_access_denied(err: BackendError) -> CommandError {
+    match err {
+        BackendError::ReadAccessDenied { .. } => user_error_with_hint(
+            err,
+            "The `--color-words` format supports access-restricted paths.",
+        ),
+        other => other.into(),
+    }
+}
+
 pub fn show_git_diff(
     repo: &dyn Repo,
     formatter: &mut dyn Formatter,
     num_context_lines: usize,
     tree_diff: TreeDiffStream,
-) -> Result<(), DiffRenderError> {
+) -> Result<(), CommandError> {
     formatter.push_label("diff")?;
 
     let mut diff_stream = materialized_diff_stream(repo.store(), tree_diff);
     async {
         while let Some((path, diff)) = diff_stream.next().await {
             let path_string = path.as_internal_file_string();
-            let (left_value, right_value) = diff?;
+            let (left_value, right_value) = diff.map_err(map_access_denied)?;
             if left_value.is_absent() {
                 let right_part = git_diff_part(&path, right_value)?;
                 formatter.with_label("file_header", |formatter| {
@@ -895,7 +919,7 @@ pub fn show_git_diff(
                 show_unified_diff_hunks(formatter, &left_part.content, &[], num_context_lines)?;
             }
         }
-        Ok::<(), DiffRenderError>(())
+        Ok::<(), CommandError>(())
     }
     .block_on()?;
     formatter.pop_label()?;
@@ -967,15 +991,15 @@ pub fn show_diff_stat(
     tree_diff: TreeDiffStream,
     path_converter: &RepoPathUiConverter,
     display_width: usize,
-) -> Result<(), DiffRenderError> {
+) -> Result<(), CommandError> {
     let mut stats: Vec<DiffStat> = vec![];
     let mut max_path_width = 0;
     let mut max_diffs = 0;
 
     let mut diff_stream = materialized_diff_stream(repo.store(), tree_diff);
     async {
         while let Some((repo_path, diff)) = diff_stream.next().await {
-            let (left, right) = diff?;
+            let (left, right) = diff.map_err(map_access_denied)?;
             let path = path_converter.format_file_path(&repo_path);
             let left_content = diff_content(&repo_path, left)?;
             let right_content = diff_content(&repo_path, right)?;
@@ -984,7 +1008,7 @@ pub fn show_diff_stat(
             max_diffs = max(max_diffs, stat.added + stat.removed);
             stats.push(stat);
         }
-        Ok::<(), DiffRenderError>(())
+        Ok::<(), CommandError>(())
     }
     .block_on()?;
 

cli/tests/runner.rs

@@ -9,6 +9,7 @@ fn test_no_forgotten_test_files() {
 }
 
 mod test_abandon_command;
+mod test_acls;
 mod test_advance_branches;
 mod test_alias;
 mod test_branch_command;

cli/tests/test_acls.rs

@@ -0,0 +1,112 @@
+// Copyright 2024 The Jujutsu Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use std::path::Path;
+
+use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
+
+fn switch_to_secret_backend(repo_path: &Path) {
+    std::fs::write(
+        repo_path
+            .join(".jj")
+            .join("repo")
+            .join("store")
+            .join("type"),
+        "secret",
+    )
+    .unwrap();
+}
+
+#[test]
+fn test_diff() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    std::fs::create_dir(repo_path.join("dir")).unwrap();
+    std::fs::write(repo_path.join("a-first"), "foo\n").unwrap();
+    std::fs::write(repo_path.join("deleted-secret"), "foo\n").unwrap();
+    std::fs::write(repo_path.join("dir").join("secret"), "foo\n").unwrap();
+    std::fs::write(repo_path.join("modified-secret"), "foo\n").unwrap();
+    std::fs::write(repo_path.join("z-last"), "foo\n").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["new"]);
+    std::fs::write(repo_path.join("a-first"), "bar\n").unwrap();
+    std::fs::remove_file(repo_path.join("deleted-secret")).unwrap();
+    std::fs::write(repo_path.join("added-secret"), "bar\n").unwrap();
+    std::fs::write(repo_path.join("dir").join("secret"), "bar\n").unwrap();
+    std::fs::write(repo_path.join("modified-secret"), "bar\n").unwrap();
+    std::fs::write(repo_path.join("z-last"), "bar\n").unwrap();
+
+    switch_to_secret_backend(&repo_path);
+
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--color-words"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    Modified regular file a-first:
+       1    1: foobar
+    Access denied to added-secret: No access
+    Access denied to deleted-secret: No access
+    Access denied to dir/secret: No access
+    Access denied to modified-secret: No access
+    Modified regular file z-last:
+       1    1: foobar
+    "###);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--summary"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    M a-first
+    A added-secret
+    D deleted-secret
+    M dir/secret
+    M modified-secret
+    M z-last
+    "###);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--types"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    FF a-first
+    -F added-secret
+    F- deleted-secret
+    FF dir/secret
+    FF modified-secret
+    FF z-last
+    "###);
+    let assert = test_env
+        .jj_cmd(&repo_path, &["diff", "--stat"])
+        .assert()
+        .failure();
+    insta::assert_snapshot!(get_stdout_string(&assert).replace('\\', "/"), @"");
+    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
+    Error: Access denied to read object 5716ca5987cbf97d6bb54920bea6adde242d87e6 of type file
+    Caused by: No access
+    Hint: The `--color-words` format supports access-restricted paths.
+    "###);
+    let assert = test_env
+        .jj_cmd(&repo_path, &["diff", "--git"])
+        .assert()
+        .failure();
+    insta::assert_snapshot!(get_stdout_string(&assert).replace('\\', "/"), @r###"
+    diff --git a/a-first b/a-first
+    index 257cc5642c...5716ca5987 100644
+    --- a/a-first
+    +++ b/a-first
+    @@ -1,1 +1,1 @@
+    -foo
+    +bar
+    "###);
+    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
+    Error: Access denied to read object 5716ca5987cbf97d6bb54920bea6adde242d87e6 of type file
+    Caused by: No access
+    Hint: The `--color-words` format supports access-restricted paths.
+    "###);
+
+    // TODO: Test external tool
+}

lib/src/backend.rs

@@ -193,6 +193,12 @@ pub enum BackendError {
         hash: String,
         source: Box<dyn std::error::Error + Send + Sync>,
     },
+    #[error("Access denied to read object {hash} of type {object_type}")]
+    ReadAccessDenied {
+        object_type: &'static str,
+        hash: String,
+        source: Box<dyn std::error::Error + Send + Sync>,
+    },
     #[error("Could not write object of type {object_type}")]
     WriteObject {
         object_type: &'static str,

lib/src/lib.rs

@@ -71,6 +71,8 @@ pub mod revset;
 pub mod revset_graph;
 mod revset_parser;
 pub mod rewrite;
+#[cfg(feature = "testing")]
+pub mod secret_backend;
 pub mod settings;
 pub mod signing;
 pub mod simple_op_heads_store;

lib/src/local_working_copy.rs

@@ -835,7 +835,7 @@ impl TreeState {
             let tree = self.current_tree().unwrap();
             let tree_paths: HashSet<_> = tree
                 .entries_matching(sparse_matcher.as_ref())
-                .map(|(path, _)| path)
+                .filter_map(|(path, result)| result.is_ok().then_some(path))
                 .collect();
             let file_states = self.file_states.all();
             let state_paths: HashSet<_> = file_states.paths().map(|path| path.to_owned()).collect();

lib/src/repo.rs

@@ -60,7 +60,7 @@ use crate::store::Store;
 use crate::submodule_store::SubmoduleStore;
 use crate::transaction::Transaction;
 use crate::view::View;
-use crate::{backend, dag_walk, op_store, revset};
+use crate::{backend, dag_walk, op_store, revset, secret_backend};
 
 pub trait Repo {
     fn store(&self) -> &Arc<Store>;
@@ -383,6 +383,15 @@ impl Default for StoreFactories {
             GitBackend::name(),
             Box::new(|settings, store_path| Ok(Box::new(GitBackend::load(settings, store_path)?))),
         );
+        #[cfg(feature = "testing")]
+        factories.add_backend(
+            secret_backend::SecretBackend::name(),
+            Box::new(|settings, store_path| {
+                Ok(Box::new(secret_backend::SecretBackend::load(
+                    settings, store_path,
+                )?))
+            }),
+        );
 
         // OpStores
         factories.add_op_store(