[clangd] Only allow remote index to be enabled from user config.

[kadircet]

thanks, lgtm!

clang-tools-extra/clangd/ConfigProvider.cpp

75

please fix

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

328

we always expect this message, as the check is performed at entry, right? (and we don't really emit diags for the other case anymore)

333

please fix

clang-tools-extra/clangd/ConfigCompile.cpp

struct FragmentCompiler {
   FragmentCompiler(CompiledFragmentImpl &Out, DiagnosticCallback D,
                    llvm::SourceMgr *SM)
       : Out(Out), Diagnostic(D), SourceMgr(SM) {}
   CompiledFragmentImpl &Out;
   DiagnosticCallback Diagnostic;
   llvm::SourceMgr *SourceMgr;
   // Normalized Fragment::SourceInfo::Directory.
   std::string FragmentDirectory;
+  bool Trusted = false;
 
   llvm::Optional<llvm::Regex>
   compileRegex(const Located<std::string> &Text,
                llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags) {
     std::string Anchored = "^(" + *Text + ")$";
     llvm::Regex Result(Anchored, Flags);
     std::string RegexError;
     if (!Result.isValid(RegexError)) {
struct FragmentCompiler {
   //    .value();
   template <typename T>
   EnumSwitch<T> compileEnum(llvm::StringRef EnumName,
                             const Located<std::string> &In) {
     return EnumSwitch<T>(EnumName, In, *this);
   }
 
   void compile(Fragment &&F) {
+    Trusted = F.Source.Trusted;
     if (!F.Source.Directory.empty()) {
       FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory);
       if (FragmentDirectory.back() != '/')
         FragmentDirectory += '/';
     }
     compile(std::move(F.If));
     compile(std::move(F.CompileFlags));
     compile(std::move(F.Index));
if (F.Background) {
             [Val](const Params &, Config &C) { C.Index.Background = *Val; });
     }
     if (F.External)
       compile(std::move(**F.External), F.External->Range);
   }
 
   void compile(Fragment::IndexBlock::ExternalBlock &&External,
                llvm::SMRange BlockRange) {
+    if (External.Server && !Trusted) {
+      diag(Error,
+           "Remote index may not be specified by untrusted configuration. "
+           "Copy this into user config to use it.",
+           External.Server->Range);
+      return;
+    }
 #ifndef CLANGD_ENABLE_REMOTE
     if (External.Server) {
       elog("Clangd isn't compiled with remote index support, ignoring Server: "
            "{0}",
            *External.Server);
       External.Server.reset();
     }
 #endif
CompiledFragment Fragment::compile(DiagnosticCallback D) && {
   if (auto *SM = Source.Manager.get()) {
     unsigned BufID = SM->getMainFileID();
     LineCol = SM->getLineAndColumn(Source.Location, BufID);
     ConfigFile = SM->getBufferInfo(BufID).Buffer->getBufferIdentifier();
   }
   trace::Span Tracer("ConfigCompile");
   SPAN_ATTACH(Tracer, "ConfigFile", ConfigFile);
   auto Result = std::make_shared<CompiledFragmentImpl>();
-  vlog("Config fragment: compiling {0}:{1} -> {2}", ConfigFile, LineCol.first,
-       Result.get());
+  vlog("Config fragment: compiling {0}:{1} -> {2} (trusted={3})", ConfigFile,
+       LineCol.first, Result.get(), Source.Trusted);
 
   FragmentCompiler{*Result, D, Source.Manager.get()}.compile(std::move(*this));
   // Return as cheaply-copyable wrapper.
   return [Result(std::move(Result))](const Params &P, Config &C) {
     return (*Result)(P, C);
   };
 }
 
 } // namespace config
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/ConfigFragment.h

struct SourceInfo {
     /// May be null, then all locations should be ignored.
     std::shared_ptr<llvm::SourceMgr> Manager;
     /// The start of the original source for this fragment.
     /// Only valid if SourceManager is set.
     llvm::SMLoc Location;
     /// Absolute path to directory the fragment is associated with. Relative
     /// paths mentioned in the fragment are resolved against this.
     std::string Directory;
+    /// Whether this fragment is allowed to make critical security/privacy
+    /// decisions.
+    bool Trusted = false;
   };
   SourceInfo Source;
 
   /// Conditions in the If block restrict when a Fragment applies.
   ///
   /// Each separate condition must match (combined with AND).
   /// When one condition has multiple values, any may match (combined with OR).
   /// e.g. `PathMatch: [foo/.*, bar/.*]` matches files in either directory.

clang-tools-extra/clangd/ConfigProvider.h

 class Provider {
 public:
   virtual ~Provider() = default;
 
   /// Reads fragments from a single YAML file with a fixed path. If non-empty,
   /// Directory will be used to resolve relative paths in the fragments.
   static std::unique_ptr<Provider> fromYAMLFile(llvm::StringRef AbsPath,
                                                 llvm::StringRef Directory,
-                                                const ThreadsafeFS &);
+                                                const ThreadsafeFS &,
+                                                bool Trusted = false);
   // Reads fragments from YAML files found relative to ancestors of Params.Path.
   //
   // All fragments that exist are returned, starting from distant ancestors.
   // For instance, given RelPath of ".clangd", then for source file /foo/bar.cc,
   // the searched fragments are [/.clangd, /foo/.clangd].
   //
   // If Params does not specify a path, no fragments are returned.
   static std::unique_ptr<Provider>
-  fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, const ThreadsafeFS &);
+  fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, const ThreadsafeFS &,
+                                bool Trusted = false);
 
   /// A provider that includes fragments from all the supplied providers.
   /// Order is preserved; later providers take precedence over earlier ones.
   static std::unique_ptr<Provider> combine(std::vector<const Provider *>);
 
   /// Build a config based on this provider.
   Config getConfig(const Params &, DiagnosticCallback) const;
 

clang-tools-extra/clangd/ConfigProvider.cpp

class FileConfigCache : public FileCache {
   mutable llvm::SmallVector<CompiledFragment, 1> CachedValue;
   std::string Directory;
 
 public:
   FileConfigCache(llvm::StringRef Path, llvm::StringRef Directory)
       : FileCache(Path), Directory(Directory) {}
 
   void get(const ThreadsafeFS &TFS, DiagnosticCallback DC,
-           std::chrono::steady_clock::time_point FreshTime,
+           std::chrono::steady_clock::time_point FreshTime, bool Trusted,
            std::vector<CompiledFragment> &Out) const {
     read(
         TFS, FreshTime,
         [&](llvm::Optional<llvm::StringRef> Data) {
           CachedValue.clear();
           if (Data)
             for (auto &Fragment : Fragment::parseYAML(*Data, path(), DC)) {
               Fragment.Source.Directory = Directory;
+              Fragment.Source.Trusted = Trusted;
               CachedValue.push_back(std::move(Fragment).compile(DC));
             }
         },
         [&]() { llvm::copy(CachedValue, std::back_inserter(Out)); });
   }
 };
 
 std::unique_ptr<Provider> Provider::fromYAMLFile(llvm::StringRef AbsPath,
                                                  llvm::StringRef Directory,
-                                                 const ThreadsafeFS &FS) {
+                                                 const ThreadsafeFS &FS,
+                                                 bool Trusted) {
   class AbsFileProvider : public Provider {
     mutable FileConfigCache Cache; // threadsafe
     const ThreadsafeFS &FS;
+    bool Trusted;
 
     std::vector<CompiledFragment>
     getFragments(const Params &P, DiagnosticCallback DC) const override {
       std::vector<CompiledFragment> Result;
-      Cache.get(FS, DC, P.FreshTime, Result);
+      Cache.get(FS, DC, P.FreshTime, Trusted, Result);
       return Result;
     };
 
   public:
     AbsFileProvider(llvm::StringRef Path, llvm::StringRef Directory,
-                    const ThreadsafeFS &FS)
-        : Cache(Path, Directory), FS(FS) {
+                    const ThreadsafeFS &FS, bool Trusted)
+        : Cache(Path, Directory), FS(FS), Trusted(Trusted) {
       assert(llvm::sys::path::is_absolute(Path));
     }
   };
 
-  return std::make_unique<AbsFileProvider>(AbsPath, Directory, FS);
+  return std::make_unique<AbsFileProvider>(AbsPath, Directory, FS, Trusted);
 }
 
 std::unique_ptr<Provider>
 Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath,
-                                        const ThreadsafeFS &FS) {
+                                        const ThreadsafeFS &FS, bool Trusted) {
   class RelFileProvider : public Provider {
     std::string RelPath;
     const ThreadsafeFS &FS;
+    bool Trusted;
 
     mutable std::mutex Mu;
     // Keys are the (posix-style) ancestor directory, not the config within it.
     // We only insert into this map, so pointers to values are stable forever.
     // Mutex guards the map itself, not the values (which are threadsafe).
     mutable llvm::StringMap<FileConfigCache> Cache;
 
     std::vector<CompiledFragment>
getFragments(const Params &P, DiagnosticCallback DC) const override {
           }
           Caches.push_back(&It->second);
         }
       }
       // Finally query each individual file.
       // This will take a (per-file) lock for each file that actually exists.
       std::vector<CompiledFragment> Result;
       for (FileConfigCache *Cache : llvm::reverse(Caches))
-        Cache->get(FS, DC, P.FreshTime, Result);
+        Cache->get(FS, DC, P.FreshTime, Trusted, Result);
       return Result;
     };
 
   public:
-    RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS)
-        : RelPath(RelPath), FS(FS) {
+    RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS,
+                    bool Trusted)
+        : RelPath(RelPath), FS(FS), Trusted(Trusted) {
       assert(llvm::sys::path::is_relative(RelPath));
     }
   };
 
-  return std::make_unique<RelFileProvider>(RelPath, FS);
+  return std::make_unique<RelFileProvider>(RelPath, FS, Trusted);
 }
 
 std::unique_ptr<Provider>
 Provider::combine(std::vector<const Provider *> Providers) {
   class CombinedProvider : public Provider {
     std::vector<const Provider *> Providers;
 
     std::vector<CompiledFragment>

clang-tools-extra/clangd/tool/ClangdMain.cpp

#endif
   std::unique_ptr<config::Provider> Config;
   if (EnableConfig) {
     ProviderStack.push_back(
         config::Provider::fromAncestorRelativeYAMLFiles(".clangd", TFS));
     llvm::SmallString<256> UserConfig;
     if (llvm::sys::path::user_config_directory(UserConfig)) {
       llvm::sys::path::append(UserConfig, "clangd", "config.yaml");
       vlog("User config file is {0}", UserConfig);
-      ProviderStack.push_back(
-          config::Provider::fromYAMLFile(UserConfig, /*Directory=*/"", TFS));
+      ProviderStack.push_back(config::Provider::fromYAMLFile(
+          UserConfig, /*Directory=*/"", TFS, /*Trusted=*/true));
     } else {
       elog("Couldn't determine user config file, not loading");
     }
   }
   ProviderStack.push_back(std::make_unique<FlagsConfigProvider>());
   std::vector<const config::Provider *> ProviderPointers;
   for (const auto &P : ProviderStack)
     ProviderPointers.push_back(P.get());

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

EXPECT_THAT(
       ElementsAre(
           AllOf(DiagMessage("clang-tidy check 'unknown-check' was not found"),
                 DiagKind(llvm::SourceMgr::DK_Warning)),
           AllOf(
               DiagMessage("clang-tidy check 'llvm-includeorder' was not found"),
               DiagKind(llvm::SourceMgr::DK_Warning))));
 }
 
+TEST_F(ConfigCompileTests, ExternalServerNeedsTrusted) {
+  Fragment::IndexBlock::ExternalBlock External;
+  External.Server.emplace("xxx");
+  Frag.Index.External = std::move(External);
+  compileAndApply();
+  EXPECT_THAT(
+      Diags.Diagnostics,
+      ElementsAre(DiagMessage(
+          "Remote index may not be specified by untrusted configuration. "
+          "Copy this into user config to use it.")));
+  EXPECT_FALSE(Conf.Index.External.hasValue());
+}
+
 TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
+  Frag.Source.Trusted = true;
   Fragment::IndexBlock::ExternalBlock External;
   External.File.emplace("");
   External.Server.emplace("");
   Frag.Index.External = std::move(External);
   compileAndApply();
 #ifdef CLANGD_ENABLE_REMOTE
   EXPECT_THAT(
       Diags.Diagnostics,