Dev #197
Merged
Dev #197
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Chore/vendor
blar-app
bot
added
size XL
PR SummaryThis pull request focuses on the integration and restructuring of the Key Findings
Pull Request Impact: 108 🔄 File Changes Overview
📃 Unreviewed filesThe following files were not reviewed by the agent:
📊 Impact SummaryThis tables shows the impact of the changes in the codebase
📜 Blar InstructionsBlar Commands
Tags Explanation
|
![blar-app[bot]](https://avatars.githubusercontent.com/in/940296?s=60&v=4){kind=small}
Comment on lines
+84
to
+106
| def _get_initialize_params(self, repository_absolute_path: str) -> InitializeParams: | ||
| """ | ||
| Returns the initialize params for the TypeScript Language Server. | ||
| """ | ||
| with open(os.path.join(os.path.dirname(__file__), "initialize_params.json"), "r") as f: | ||
| d = json.load(f) | ||
|
|
||
| del d["_description"] | ||
|
|
||
| d["processId"] = os.getpid() | ||
| assert d["rootPath"] == "$rootPath" | ||
| d["rootPath"] = repository_absolute_path | ||
|
|
||
| assert d["rootUri"] == "$rootUri" | ||
| d["rootUri"] = pathlib.Path(repository_absolute_path).as_uri() | ||
|
|
||
| assert d["workspaceFolders"][0]["uri"] == "$uri" | ||
| d["workspaceFolders"][0]["uri"] = pathlib.Path(repository_absolute_path).as_uri() | ||
|
|
||
| assert d["workspaceFolders"][0]["name"] == "$name" | ||
| d["workspaceFolders"][0]["name"] = os.path.basename(repository_absolute_path) | ||
|
|
||
| return d |
There was a problem hiding this comment.
🟠 Warning 🐛 Bug
- The method _get_initialize_params in gopls.py asserts the presence of specific placeholders in the JSON file before replacing them.
- The placeholders checked are '$rootPath', '$rootUri', '$uri', and '$name'.
- If these placeholders do not match the hardcoded values exactly or if the JSON content changes, a Runtime AssertionError will occur.
- This tight coupling between the JSON content and hardcoded values can lead to potential runtime issues when the JSON structure is modified.
with open(os.path.join(os.path.dirname(__file__), "initialize_params.json"), "r") as f:
d = json.load(f)
del d["_description"]
d["processId"] = os.getpid()
assert d["rootPath"] == "$rootPath"
d["rootPath"] = repository_absolute_path
assert d["rootUri"] == "$rootUri"
d["rootUri"] = pathlib.Path(repository_absolute_path).as_uri()
assert d["workspaceFolders"][0]["uri"] == "$uri"
d["workspaceFolders"][0]["uri"] = pathlib.Path(repository_absolute_path).as_uri()
assert d["workspaceFolders"][0]["name"] == "$name"
d["workspaceFolders"][0]["name"] = os.path.basename(repository_absolute_path)
Comment on lines
+67
to
+80
| slnfilename = find_least_depth_sln_file(repository_root_path) | ||
| if slnfilename is None: | ||
| logger.log("No *.sln file found in repository", logging.ERROR) | ||
| raise MultilspyException("No SLN file found in repository") | ||
|
|
||
| cmd = " ".join( | ||
| [ | ||
| omnisharp_executable_path, | ||
| "-lsp", | ||
| "--encoding", | ||
| "ascii", | ||
| "-z", | ||
| "-s", | ||
| slnfilename, |
There was a problem hiding this comment.
🔴 Error 🛡️ Cyber Security
- The value of
slnfilenameis obtained from the functionfind_least_depth_sln_file(repository_root_path), which scans the repository for solution files. - This value is concatenated directly into the command string without any sanitization.
- Using unsanitized input from potentially user-controlled file names poses a security risk, specifically a command injection vulnerability when the command is executed.
Here is the relevant code snippet:
slnfilename = find_least_depth_sln_file(repository_root_path)
if slnfilename is None:
logger.log("No *.sln file found in repository", logging.ERROR)
raise MultilspyException("No SLN file found in repository")
cmd = " ".join(
[
omnisharp_executable_path,
"-lsp",
"--encoding",
"ascii",
"-z",
"-s",
slnfilename,
"--hostPID",
str(os.getpid()),
"DotNet:enablePackageRestore=false",
"--loglevel",
"trace",
"--plugin",
dll_path,
"FileOptions:SystemExcludeSearchPatterns:0=**/.git",
"FileOptions:SystemExcludeSearchPatterns:1=**/.svn",
"FileOptions:SystemExcludeSearchPatterns:2=**/.hg",
"FileOptions:SystemExcludeSearchPatterns:3=**/CVS",
"FileOptions:SystemExcludeSearchPatterns:4=**/.DS_Store",
"FileOptions:SystemExcludeSearchPatterns:5=**/Thumbs.db",
"RoslynExtensionsOptions:EnableAnalyzersSupport=true",
"FormattingOptions:EnableEditorConfigSupport=true",
"RoslynExtensionsOptions:EnableImportCompletion=true",
"Sdk:IncludePrereleases=true",
"RoslynExtensionsOptions:AnalyzeOpenDocumentsOnly=true",
"formattingOptions:useTabs=false",
"formattingOptions:tabSize=4",
"formattingOptions:indentationSize=4",
]
)
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The function
check_experimental_statuscallsself.server_ready.set()whenparams['quiescent']isTrue. self.server_readyis not defined in the OmniSharp class.- This absence means that referencing
self.server_ready.set()will result in an AttributeError during execution.
Code Snippet:
async def check_experimental_status(params):
if params["quiescent"] == True:
self.server_ready.set() # self.server_ready is not defined anywhere in OmniSharp
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The method
register_capability_handlercallsself.completions_available.set()when the registration method istextDocument/completion. self.completions_availableis not defined or initialized anywhere in theOmniSharpclass.- This absence results in an
AttributeErrorwhenself.completions_available.set()is invoked. - The attributes
self.definition_availableandself.references_availableare defined and set appropriately, butself.completions_availableis missing.
Code snippet:
for registration in params["registrations"]:
if registration["method"] == "textDocument/definition":
self.definition_available.set()
if registration["method"] == "textDocument/references":
self.references_available.set()
if registration["method"] == "textDocument/completion":
self.completions_available.set()
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The _get_initialize_params method includes four assert statements:
- assert d["rootPath"] == "$rootPath"
- assert d["rootUri"] == "$rootUri"
- assert d["workspaceFolders"][0]["uri"] == "$uri"
- assert d["workspaceFolders"][0]["name"] == "$name"
- These assertions explicitly check that specific JSON fields match expected placeholder strings.
- If 'initialize_params.json' does not contain these exact values, AssertionErrors will be raised.
- Other parts of the code rely on the modified initialization parameters after these checks.
Comment on lines
+82
to
+84
| async def check_experimental_status(params): | ||
| if params["quiescent"] == True: | ||
| self.completions_available.set() |
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The
check_experimental_statusasync function directly accessesparams['quiescent']without verifying if the key exists. - If the 'quiescent' key is absent in the
paramsdictionary, this results in aKeyError. - Implementing a check for the existence of 'quiescent' is necessary to prevent runtime errors in scenarios where the key may not be provided.
async def check_experimental_status(params):
if params["quiescent"] == True:
self.completions_available.set()
There was a problem hiding this comment.
🟠 Warning 🐛 Bug
- The
_get_initialize_paramsmethod directly accessesd['workspaceFolders'][0]. - There is no verification for the existence of the 'workspaceFolders' key.
- No checks are performed to ensure the list is non-empty before indexing.
- This can lead to an IndexError if 'workspaceFolders' is absent or empty.
assert d["workspaceFolders"][0]["uri"] == "$uri"
d["workspaceFolders"][0]["uri"] = pathlib.Path(repository_absolute_path).as_uri()
assert d["workspaceFolders"][0]["name"] == "$name"
d["workspaceFolders"][0]["name"] = os.path.basename(repository_absolute_path)
Comment on lines
+120
to
+126
| async def register_capability_handler(params): | ||
| assert "registrations" in params | ||
| for registration in params["registrations"]: | ||
| if registration["method"] == "workspace/executeCommand": | ||
| self.initialize_searcher_command_available.set() | ||
| self.resolve_main_method_available.set() | ||
| return |
There was a problem hiding this comment.
🟠 Warning 🐛 Bug
- The function 'register_capability_handler' asserts that 'registrations' exists in 'params' without validating its type.
- It directly iterates over 'params["registrations"]', which may lead to runtime errors if 'registrations' is not an iterable.
- No checks are performed to ensure that 'params["registrations"]' is indeed iterable before iteration.
- Lack of type validation could result in exceptions if the expected data type is not correctly provided.
Code Snippet:
async def register_capability_handler(params):
assert "registrations" in params
for registration in params["registrations"]:
if registration["method"] == "workspace/executeCommand":
self.initialize_searcher_command_available.set()
self.resolve_main_method_available.set()
return
Comment on lines
+81
to
+103
| def _get_initialize_params(self, repository_absolute_path: str) -> InitializeParams: | ||
| """ | ||
| Returns the initialize params for the Rust Analyzer Language Server. | ||
| """ | ||
| with open(os.path.join(os.path.dirname(__file__), "initialize_params.json"), "r") as f: | ||
| d = json.load(f) | ||
|
|
||
| del d["_description"] | ||
|
|
||
| d["processId"] = os.getpid() | ||
| assert d["rootPath"] == "$rootPath" | ||
| d["rootPath"] = repository_absolute_path | ||
|
|
||
| assert d["rootUri"] == "$rootUri" | ||
| d["rootUri"] = pathlib.Path(repository_absolute_path).as_uri() | ||
|
|
||
| assert d["workspaceFolders"][0]["uri"] == "$uri" | ||
| d["workspaceFolders"][0]["uri"] = pathlib.Path(repository_absolute_path).as_uri() | ||
|
|
||
| assert d["workspaceFolders"][0]["name"] == "$name" | ||
| d["workspaceFolders"][0]["name"] = os.path.basename(repository_absolute_path) | ||
|
|
||
| return d |
There was a problem hiding this comment.
🔴 Error 🐛 Bug
The _get_initialize_params method poses a risk of raising exceptions due to several assumptions about the JSON data structure:
-
The function expects the JSON loaded from
initialize_params.jsonto contain specific placeholder values:rootPathmust be equal to'$rootPath'.rootUrimust be equal to'$rootUri'.- The first element of
workspaceFoldersmust haveuriequal to'$uri'andnameequal to'$name'.
-
The
workspaceFolderslist must contain at least one element, as the code directly accesses the first index.
If the JSON file structure is altered or misconfigured, the assert statements or direct indexing could lead to runtime errors, potentially causing the application to fail.
Here is the relevant code snippet:
def _get_initialize_params(self, repository_absolute_path: str) -> InitializeParams:
with open(os.path.join(os.path.dirname(__file__), "initialize_params.json"), "r") as f:
d = json.load(f)
del d["_description"]
d["processId"] = os.getpid()
assert d["rootPath"] == "$rootPath"
d["rootPath"] = repository_absolute_path
assert d["rootUri"] == "$rootUri"
d["rootUri"] = pathlib.Path(repository_absolute_path).as_uri()
assert d["workspaceFolders"][0]["uri"] == "$uri"
d["workspaceFolders"][0]["uri"] = pathlib.Path(repository_absolute_path).as_uri()
assert d["workspaceFolders"][0]["name"] == "$name"
d["workspaceFolders"][0]["name"] = os.path.basename(repository_absolute_path)
return d
Comment on lines
+166
to
+172
| init_response = await self.server.send.initialize(initialize_params) | ||
| assert init_response["capabilities"]["textDocumentSync"]["change"] == 2 | ||
| assert "completionProvider" in init_response["capabilities"] | ||
| assert init_response["capabilities"]["completionProvider"] == { | ||
| "resolveProvider": True, | ||
| "triggerCharacters": [":", ".", "'", "("], | ||
| "completionItem": {"labelDetailsSupport": True}, |
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The code contains hardcoded assertions that check the structure of the
init_responsefrom the server. - It asserts that
init_response["capabilities"]["textDocumentSync"]["change"]equals 2. - Additionally, it verifies that
completionProviderexists and matches a specific structure. - These checks impose strict requirements on the server's response format.
- Any deviation from the expected structure can lead to assertion failures, causing the client to terminate unexpectedly.
Here is the relevant code snippet:
init_response = await self.server.send.initialize(initialize_params)
assert init_response["capabilities"]["textDocumentSync"]["change"] == 2
assert "completionProvider" in init_response["capabilities"]
assert init_response["capabilities"]["completionProvider"] == {
"resolveProvider": True,
"triggerCharacters": [":" , ".", "'", "("],
"completionItem": {"labelDetailsSupport": True},
}
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The attributes
self.initialize_searcher_command_availableandself.resolve_main_method_availableare used in the nestedregister_capability_handlerfunction within thestart_servermethod of theSolargraphclass. - There is no evidence that these attributes are defined in the
Solargraphclass's__init__method or anywhere else in the provided code snippet. - The
LanguageServerclass, from whichSolargraphinherits, also does not define these attributes in its__init__method or any other methods. - Without being defined, their use in
register_capability_handlercan lead to anAttributeErrorat runtime. - Other attributes in the class are explicitly defined, reinforcing that these two attributes are missing from the code context.
Code snippet example:
def __init__(
self,
config: MultilspyConfig,
logger: MultilspyLogger,
repository_root_path: str,
process_launch_info: ProcessLaunchInfo,
language_id: str,
):
if type(self) == LanguageServer:
raise MultilspyException(
"LanguageServer is an abstract class and cannot be instantiated directly. Use LanguageServer.create method instead."
)
self.logger = logger
self.server_started = False
self.repository_root_path: str = repository_root_path
self.completions_available = asyncio.Event()
… # Additional attribute initialization follows, but not for the two attributes in question.
There was a problem hiding this comment.
🟠 Warning 🐛 Bug
- The
start_servermethod referencesself.completions_availablethrough a call toset()after sending the initialization request. - There is no initialization of
self.completions_availablein the__init__method or elsewhere in the provided code snippet. - This implies that if
self.completions_availableis not defined in any omitted part of the class, it may lead to anAttributeErrorwhen accessed. - The absence of evidence for initialization means that the reference to
self.completions_availableoccurs before it may potentially be initialized.
def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_root_path: str):
"""
Creates a Solargraph instance. This class is not meant to be instantiated directly.
Use LanguageServer.create() instead.
"""
solargraph_executable_path = self.setup_runtime_dependencies(logger, config, repository_root_path)
super().__init__(
config,
logger,
repository_root_path,
ProcessLaunchInfo(cmd=f"{solargraph_executable_path} stdio", cwd=repository_root_path),
"ruby",
)
self.server_ready = asyncio.Event()
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The function
setup_runtime_dependenciesdeletes the_descriptionkey from the loaded JSON object before checking the content of theruntimeDependencieskey. - Following the deletion, the code accesses the first element of
runtimeDependencieswithout verifying if the list is non-empty. - This can lead to potential
KeyErrorif the_descriptionkey is missing in the JSON, or anIndexErrorifruntimeDependenciesis an empty array.
with open(os.path.join(os.path.dirname(__file__), "runtime_dependencies.json"), "r") as f:
d = json.load(f)
del d["_description"]
dependency = d["runtimeDependencies"][0]
There was a problem hiding this comment.
🔴 Error 🐛 Bug
- The method
start_server()includes a call toself.completions_available.set()within the inner functionregister_capability_handler. - The attribute
completions_availableis not defined or initialized anywhere in theEclipseJDTLSclass. - This will lead to an
AttributeErrorat runtime sincecompletions_availableis referenced without prior initialization. - Other attributes such as
service_ready_eventandintellicode_enable_command_availableare properly initialized in the class constructor, indicating an inconsistency in attribute management.
Code Snippet:
if registration["method"] == "textDocument/completion":
assert registration["registerOptions"]["resolveProvider"] == True
assert registration["registerOptions"]["triggerCharacters"] == [".", "@", "#", "*", " "]
self.completions_available.set()
rysweet
pushed a commit
to rysweet/cue
that referenced
this pull request
Aug 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.