[llvm-branch-commits] [clang-tools-extra] 84e8b1c - [clangd] Only allow remote index to be enabled from user config.
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Jun 8 21:30:33 PDT 2021
Author: Sam McCall
Date: 2021-06-08T21:30:01-07:00
New Revision: 84e8b1cf07b9845ea8e1e07ed5ccc3c5a70d975b
URL: https://github.com/llvm/llvm-project/commit/84e8b1cf07b9845ea8e1e07ed5ccc3c5a70d975b
DIFF: https://github.com/llvm/llvm-project/commit/84e8b1cf07b9845ea8e1e07ed5ccc3c5a70d975b.diff
LOG: [clangd] Only allow remote index to be enabled from user config.
Differential Revision: https://reviews.llvm.org/D100542
(cherry picked from commit ecf93a716c9ecf2e38898547df90323e239a623c)
Added:
Modified:
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/ConfigProvider.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index b4f0d61868863..7d5466778a81d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -101,6 +101,7 @@ struct FragmentCompiler {
llvm::SourceMgr *SourceMgr;
// Normalized Fragment::SourceInfo::Directory.
std::string FragmentDirectory;
+ bool Trusted = false;
llvm::Optional<llvm::Regex>
compileRegex(const Located<std::string> &Text,
@@ -183,6 +184,7 @@ struct FragmentCompiler {
}
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() != '/')
@@ -319,6 +321,13 @@ struct FragmentCompiler {
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: "
@@ -489,8 +498,8 @@ CompiledFragment Fragment::compile(DiagnosticCallback D) && {
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.
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index c36b07f5e8e22..c98ca3a2dd522 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -94,6 +94,9 @@ struct Fragment {
/// 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;
diff --git a/clang-tools-extra/clangd/ConfigProvider.cpp b/clang-tools-extra/clangd/ConfigProvider.cpp
index 05b2ba50566dd..6dfb00b14fc6c 100644
--- a/clang-tools-extra/clangd/ConfigProvider.cpp
+++ b/clang-tools-extra/clangd/ConfigProvider.cpp
@@ -34,7 +34,7 @@ class FileConfigCache : public FileCache {
: 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,
@@ -43,6 +43,7 @@ class FileConfigCache : public FileCache {
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));
}
},
@@ -52,35 +53,38 @@ class FileConfigCache : public FileCache {
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.
@@ -128,18 +132,19 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath,
// This will take a (per-file) lock for each file that actually exists.
std::vector<CompiledFragment> Result;
for (FileConfigCache *Cache : 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>
diff --git a/clang-tools-extra/clangd/ConfigProvider.h b/clang-tools-extra/clangd/ConfigProvider.h
index 25d9450f28a72..428438b67f14d 100644
--- a/clang-tools-extra/clangd/ConfigProvider.h
+++ b/clang-tools-extra/clangd/ConfigProvider.h
@@ -69,7 +69,8 @@ class Provider {
/// 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.
@@ -78,7 +79,8 @@ class Provider {
//
// 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.
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index fe69079bfe67c..99c3d97ce35d3 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -831,8 +831,8 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
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");
}
diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 4961d3474fd9a..a3738681bec5a 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -318,7 +318,21 @@ TEST_F(ConfigCompileTests, TidyBadChecks) {
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("");
More information about the llvm-branch-commits
mailing list