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

Differential Revision: https://reviews.llvm.org/D100542
This commit is contained in:
Sam McCall 2021-04-15 12:52:58 +02:00
parent 49cbf4cd85
commit ecf93a716c
6 changed files with 50 additions and 17 deletions

View File

@ -101,6 +101,7 @@ struct FragmentCompiler {
llvm::SourceMgr *SourceMgr; llvm::SourceMgr *SourceMgr;
// Normalized Fragment::SourceInfo::Directory. // Normalized Fragment::SourceInfo::Directory.
std::string FragmentDirectory; std::string FragmentDirectory;
bool Trusted = false;
llvm::Optional<llvm::Regex> llvm::Optional<llvm::Regex>
compileRegex(const Located<std::string> &Text, compileRegex(const Located<std::string> &Text,
@ -183,6 +184,7 @@ struct FragmentCompiler {
} }
void compile(Fragment &&F) { void compile(Fragment &&F) {
Trusted = F.Source.Trusted;
if (!F.Source.Directory.empty()) { if (!F.Source.Directory.empty()) {
FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory); FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory);
if (FragmentDirectory.back() != '/') if (FragmentDirectory.back() != '/')
@ -320,6 +322,13 @@ struct FragmentCompiler {
void compile(Fragment::IndexBlock::ExternalBlock &&External, void compile(Fragment::IndexBlock::ExternalBlock &&External,
llvm::SMRange BlockRange) { 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 #ifndef CLANGD_ENABLE_REMOTE
if (External.Server) { if (External.Server) {
elog("Clangd isn't compiled with remote index support, ignoring Server: " elog("Clangd isn't compiled with remote index support, ignoring Server: "
@ -510,8 +519,8 @@ CompiledFragment Fragment::compile(DiagnosticCallback D) && {
trace::Span Tracer("ConfigCompile"); trace::Span Tracer("ConfigCompile");
SPAN_ATTACH(Tracer, "ConfigFile", ConfigFile); SPAN_ATTACH(Tracer, "ConfigFile", ConfigFile);
auto Result = std::make_shared<CompiledFragmentImpl>(); auto Result = std::make_shared<CompiledFragmentImpl>();
vlog("Config fragment: compiling {0}:{1} -> {2}", ConfigFile, LineCol.first, vlog("Config fragment: compiling {0}:{1} -> {2} (trusted={3})", ConfigFile,
Result.get()); LineCol.first, Result.get(), Source.Trusted);
FragmentCompiler{*Result, D, Source.Manager.get()}.compile(std::move(*this)); FragmentCompiler{*Result, D, Source.Manager.get()}.compile(std::move(*this));
// Return as cheaply-copyable wrapper. // Return as cheaply-copyable wrapper.

View File

@ -94,6 +94,9 @@ struct Fragment {
/// Absolute path to directory the fragment is associated with. Relative /// Absolute path to directory the fragment is associated with. Relative
/// paths mentioned in the fragment are resolved against this. /// paths mentioned in the fragment are resolved against this.
std::string Directory; std::string Directory;
/// Whether this fragment is allowed to make critical security/privacy
/// decisions.
bool Trusted = false;
}; };
SourceInfo Source; SourceInfo Source;

View File

@ -36,7 +36,7 @@ public:
: FileCache(Path), Directory(Directory) {} : FileCache(Path), Directory(Directory) {}
void get(const ThreadsafeFS &TFS, DiagnosticCallback DC, 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 { std::vector<CompiledFragment> &Out) const {
read( read(
TFS, FreshTime, TFS, FreshTime,
@ -45,6 +45,7 @@ public:
if (Data) if (Data)
for (auto &Fragment : Fragment::parseYAML(*Data, path(), DC)) { for (auto &Fragment : Fragment::parseYAML(*Data, path(), DC)) {
Fragment.Source.Directory = Directory; Fragment.Source.Directory = Directory;
Fragment.Source.Trusted = Trusted;
CachedValue.push_back(std::move(Fragment).compile(DC)); CachedValue.push_back(std::move(Fragment).compile(DC));
} }
}, },
@ -54,35 +55,38 @@ public:
std::unique_ptr<Provider> Provider::fromYAMLFile(llvm::StringRef AbsPath, std::unique_ptr<Provider> Provider::fromYAMLFile(llvm::StringRef AbsPath,
llvm::StringRef Directory, llvm::StringRef Directory,
const ThreadsafeFS &FS) { const ThreadsafeFS &FS,
bool Trusted) {
class AbsFileProvider : public Provider { class AbsFileProvider : public Provider {
mutable FileConfigCache Cache; // threadsafe mutable FileConfigCache Cache; // threadsafe
const ThreadsafeFS &FS; const ThreadsafeFS &FS;
bool Trusted;
std::vector<CompiledFragment> std::vector<CompiledFragment>
getFragments(const Params &P, DiagnosticCallback DC) const override { getFragments(const Params &P, DiagnosticCallback DC) const override {
std::vector<CompiledFragment> Result; std::vector<CompiledFragment> Result;
Cache.get(FS, DC, P.FreshTime, Result); Cache.get(FS, DC, P.FreshTime, Trusted, Result);
return Result; return Result;
}; };
public: public:
AbsFileProvider(llvm::StringRef Path, llvm::StringRef Directory, AbsFileProvider(llvm::StringRef Path, llvm::StringRef Directory,
const ThreadsafeFS &FS) const ThreadsafeFS &FS, bool Trusted)
: Cache(Path, Directory), FS(FS) { : Cache(Path, Directory), FS(FS), Trusted(Trusted) {
assert(llvm::sys::path::is_absolute(Path)); 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> std::unique_ptr<Provider>
Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath,
const ThreadsafeFS &FS) { const ThreadsafeFS &FS, bool Trusted) {
class RelFileProvider : public Provider { class RelFileProvider : public Provider {
std::string RelPath; std::string RelPath;
const ThreadsafeFS &FS; const ThreadsafeFS &FS;
bool Trusted;
mutable std::mutex Mu; mutable std::mutex Mu;
// Keys are the (posix-style) ancestor directory, not the config within it. // Keys are the (posix-style) ancestor directory, not the config within it.
@ -124,18 +128,19 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath,
// This will take a (per-file) lock for each file that actually exists. // This will take a (per-file) lock for each file that actually exists.
std::vector<CompiledFragment> Result; std::vector<CompiledFragment> Result;
for (FileConfigCache *Cache : llvm::reverse(Caches)) for (FileConfigCache *Cache : llvm::reverse(Caches))
Cache->get(FS, DC, P.FreshTime, Result); Cache->get(FS, DC, P.FreshTime, Trusted, Result);
return Result; return Result;
}; };
public: public:
RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS) RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS,
: RelPath(RelPath), FS(FS) { bool Trusted)
: RelPath(RelPath), FS(FS), Trusted(Trusted) {
assert(llvm::sys::path::is_relative(RelPath)); 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> std::unique_ptr<Provider>

View File

@ -69,7 +69,8 @@ public:
/// Directory will be used to resolve relative paths in the fragments. /// Directory will be used to resolve relative paths in the fragments.
static std::unique_ptr<Provider> fromYAMLFile(llvm::StringRef AbsPath, static std::unique_ptr<Provider> fromYAMLFile(llvm::StringRef AbsPath,
llvm::StringRef Directory, llvm::StringRef Directory,
const ThreadsafeFS &); const ThreadsafeFS &,
bool Trusted = false);
// Reads fragments from YAML files found relative to ancestors of Params.Path. // Reads fragments from YAML files found relative to ancestors of Params.Path.
// //
// All fragments that exist are returned, starting from distant ancestors. // All fragments that exist are returned, starting from distant ancestors.
@ -78,7 +79,8 @@ public:
// //
// If Params does not specify a path, no fragments are returned. // If Params does not specify a path, no fragments are returned.
static std::unique_ptr<Provider> 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. /// A provider that includes fragments from all the supplied providers.
/// Order is preserved; later providers take precedence over earlier ones. /// Order is preserved; later providers take precedence over earlier ones.

View File

@ -855,8 +855,8 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
if (llvm::sys::path::user_config_directory(UserConfig)) { if (llvm::sys::path::user_config_directory(UserConfig)) {
llvm::sys::path::append(UserConfig, "clangd", "config.yaml"); llvm::sys::path::append(UserConfig, "clangd", "config.yaml");
vlog("User config file is {0}", UserConfig); vlog("User config file is {0}", UserConfig);
ProviderStack.push_back( ProviderStack.push_back(config::Provider::fromYAMLFile(
config::Provider::fromYAMLFile(UserConfig, /*Directory=*/"", TFS)); UserConfig, /*Directory=*/"", TFS, /*Trusted=*/true));
} else { } else {
elog("Couldn't determine user config file, not loading"); elog("Couldn't determine user config file, not loading");
} }

View File

@ -318,7 +318,21 @@ TEST_F(ConfigCompileTests, TidyBadChecks) {
DiagKind(llvm::SourceMgr::DK_Warning)))); 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) { TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
Frag.Source.Trusted = true;
Fragment::IndexBlock::ExternalBlock External; Fragment::IndexBlock::ExternalBlock External;
External.File.emplace(""); External.File.emplace("");
External.Server.emplace(""); External.Server.emplace("");