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

cli/src/command_error.rs

@@ -365,6 +365,7 @@ impl From<DiffRenderError> for CommandError {
         match err {
             DiffRenderError::DiffGenerate(_) => user_error(err),
             DiffRenderError::Backend(err) => err.into(),
+            DiffRenderError::AccessDenied { .. } => user_error(err),
             DiffRenderError::Io(err) => err.into(),
         }
     }

cli/src/commands/cat.rs

@@ -104,6 +104,13 @@ fn write_tree_entries<P: AsRef<RepoPath>>(
         let materialized = materialize_tree_value(repo.store(), path.as_ref(), value).block_on()?;
         match materialized {
             MaterializedTreeValue::Absent => panic!("absent values should be excluded"),
+            MaterializedTreeValue::AccessDenied(err) => {
+                let ui_path = workspace_command.format_file_path(path.as_ref());
+                writeln!(
+                    ui.warning_default(),
+                    "Path '{ui_path}' exists but access is denied: {err}"
+                )?;
+            }
             MaterializedTreeValue::File { mut reader, .. } => {
                 io::copy(&mut reader, &mut ui.stdout_formatter().as_mut())?;
             }
@@ -114,7 +121,7 @@ fn write_tree_entries<P: AsRef<RepoPath>>(
                 let ui_path = workspace_command.format_file_path(path.as_ref());
                 writeln!(
                     ui.warning_default(),
-                    "Path exists but is not a file: {ui_path}"
+                    "Path '{ui_path}' exists but is not a file"
                 )?;
             }
             MaterializedTreeValue::Tree(_) => panic!("entries should not contain trees"),

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

@@ -195,6 +195,11 @@ pub enum DiffRenderError {
     DiffGenerate(#[source] DiffGenerateError),
     #[error(transparent)]
     Backend(#[from] BackendError),
+    #[error("Access denied to {path}: {source}")]
+    AccessDenied {
+        path: String,
+        source: Box<dyn std::error::Error + Send + Sync>,
+    },
     #[error(transparent)]
     Io(#[from] io::Error),
 }
@@ -437,6 +442,10 @@ fn file_content_for_diff(reader: &mut dyn io::Read) -> io::Result<FileContent> {
 fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result<FileContent> {
     match value {
         MaterializedTreeValue::Absent => Ok(FileContent::empty()),
+        MaterializedTreeValue::AccessDenied(err) => Ok(FileContent {
+            is_binary: false,
+            contents: format!("Access denied: {err}").into_bytes(),
+        }),
         MaterializedTreeValue::File { mut reader, .. } => {
             file_content_for_diff(&mut reader).map_err(Into::into)
         }
@@ -469,6 +478,7 @@ fn basic_diff_file_type(value: &MaterializedTreeValue) -> &'static str {
         MaterializedTreeValue::Absent => {
             panic!("absent path in diff");
         }
+        MaterializedTreeValue::AccessDenied(_) => "access denied",
         MaterializedTreeValue::File { executable, .. } => {
             if *executable {
                 "executable file"
@@ -496,6 +506,19 @@ pub fn show_color_words_diff(
         while let Some((path, diff)) = diff_stream.next().await {
             let ui_path = path_converter.format_file_path(&path);
             let (left_value, right_value) = diff?;
+
+            match (&left_value, &right_value) {
+                (_, MaterializedTreeValue::AccessDenied(source))
+                | (MaterializedTreeValue::AccessDenied(source), _) => {
+                    write!(
+                        formatter.labeled("access-denied"),
+                        "Access denied to {ui_path}:"
+                    )?;
+                    writeln!(formatter, " {source}")?;
+                    continue;
+                }
+                _ => {}
+            }
             if left_value.is_absent() {
                 let description = basic_diff_file_type(&right_value);
                 writeln!(
@@ -610,14 +633,23 @@ struct GitDiffPart {
     content: Vec<u8>,
 }
 
-fn git_diff_part(path: &RepoPath, value: MaterializedTreeValue) -> io::Result<GitDiffPart> {
+fn git_diff_part(
+    path: &RepoPath,
+    value: MaterializedTreeValue,
+) -> Result<GitDiffPart, DiffRenderError> {
     let mode;
     let hash;
     let mut contents: Vec<u8>;
     match value {
         MaterializedTreeValue::Absent => {
             panic!("Absent path {path:?} in diff should have been handled by caller");
         }
+        MaterializedTreeValue::AccessDenied(err) => {
+            return Err(DiffRenderError::AccessDenied {
+                path: path.as_internal_file_string().to_owned(),
+                source: err,
+            });
+        }
         MaterializedTreeValue::File {
             id,
             executable,

cli/src/merge_tools/builtin.rs

@@ -137,6 +137,14 @@ fn read_file_contents(
             file_mode: scm_record::FileMode::absent(),
             contents: FileContents::Absent,
         }),
+        MaterializedTreeValue::AccessDenied(err) => Ok(FileInfo {
+            file_mode: scm_record::FileMode(mode::NORMAL),
+            contents: FileContents::Text {
+                contents: format!("Access denied: {err}"),
+                hash: None,
+                num_bytes: 0,
+            },
+        }),
 
         MaterializedTreeValue::File {
             id,

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,121 @@
+// 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 jj_lib::secret_backend::SecretBackend;
+
+use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
+
+#[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();
+
+    SecretBackend::adopt_git_repo(&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 stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    a-first         | 2 +-
+    added-secret    | 1 +
+    deleted-secret  | 1 -
+    dir/secret      | 0
+    modified-secret | 0
+    z-last          | 2 +-
+    6 files changed, 3 insertions(+), 3 deletions(-)
+    "###);
+    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 added-secret: No access
+    Caused by: No access
+    "###);
+
+    // TODO: Test external tool
+}
+
+#[test]
+fn test_cat() {
+    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::write(repo_path.join("a-first"), "foo\n").unwrap();
+    std::fs::write(repo_path.join("secret"), "bar\n").unwrap();
+    std::fs::write(repo_path.join("z-last"), "baz\n").unwrap();
+
+    SecretBackend::adopt_git_repo(&repo_path);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["cat", "."]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    foo
+    baz
+    "###);
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Warning: Path 'secret' exists but access is denied: No access
+    "###);
+}

cli/tests/test_cat_command.rs

@@ -118,6 +118,6 @@ fn test_cat_symlink() {
     a
     "###);
     insta::assert_snapshot!(stderr, @r###"
-    Warning: Path exists but is not a file: symlink1
+    Warning: Path 'symlink1' exists but is not a file
     "###);
 }

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: String,
+        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/conflicts.rs

@@ -21,7 +21,7 @@ use futures::StreamExt;
 use itertools::Itertools;
 use regex::bytes::Regex;
 
-use crate::backend::{BackendResult, CommitId, FileId, SymlinkId, TreeId, TreeValue};
+use crate::backend::{BackendError, BackendResult, CommitId, FileId, SymlinkId, TreeId, TreeValue};
 use crate::diff::{find_line_ranges, Diff, DiffHunk};
 use crate::files;
 use crate::files::{ContentHunk, MergeResult};
@@ -127,6 +127,7 @@ pub async fn materialize(
 /// e.g. the working copy or in a diff.
 pub enum MaterializedTreeValue {
     Absent,
+    AccessDenied(Box<dyn std::error::Error + Send + Sync>),
     File {
         id: FileId,
         executable: bool,
@@ -161,6 +162,19 @@ pub async fn materialize_tree_value(
     store: &Store,
     path: &RepoPath,
     value: MergedTreeValue,
+) -> BackendResult<MaterializedTreeValue> {
+    match materialize_tree_value_no_access_denied(store, path, value).await {
+        Err(BackendError::ReadAccessDenied { source, .. }) => {
+            Ok(MaterializedTreeValue::AccessDenied(source))
+        }
+        result => result,
+    }
+}
+
+async fn materialize_tree_value_no_access_denied(
+    store: &Store,
+    path: &RepoPath,
+    value: MergedTreeValue,
 ) -> BackendResult<MaterializedTreeValue> {
     match value.into_resolved() {
         Ok(None) => Ok(MaterializedTreeValue::Absent),

lib/src/lib.rs

@@ -71,6 +71,8 @@ pub mod repo_path;
 pub mod revset;
 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();
@@ -1368,7 +1368,7 @@ impl TreeState {
             }
             // TODO: Check that the file has not changed before overwriting/removing it.
             let file_state = match after {
-                MaterializedTreeValue::Absent => {
+                MaterializedTreeValue::Absent | MaterializedTreeValue::AccessDenied(_) => {
                     let mut parent_dir = disk_path.parent().unwrap();
                     loop {
                         if fs::remove_dir(parent_dir).is_err() {

lib/src/repo.rs

@@ -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(
+            crate::secret_backend::SecretBackend::name(),
+            Box::new(|settings, store_path| {
+                Ok(Box::new(crate::secret_backend::SecretBackend::load(
+                    settings, store_path,
+                )?))
+            }),
+        );
 
         // OpStores
         factories.add_op_store(