Fix and triage CodeQL/Semgrep code-scanning alerts - #1049
Merged
Merged
Conversation
- Reject whitespace and leading '@' in extension manifest values that are rendered unquoted into command templates (emscriptenLinkFlags, aaptExtraPackages, excludeLibs, ...), since the rendered command is split on whitespace before ProcessBuilder. - Reject uploaded filenames where a whitespace-delimited token starts with '-' or '@'. - Replace the ambiguous FILENAME_RE alternation with an equivalent regex that cannot backtrack exponentially. - Allow-list platform and sdkVersion path variables in buildEngineAsync. - Validate baseVariant before it is used as a path component. - Create the SDK download temp file with owner-only permissions. - Neutralize line breaks and control characters when logging a template that failed to render.
- Use Files.createDirectories where a silently failing mkdir would be swallowed later (AsyncBuilder result dir, javac class dirs, R.java dir, packages dir, remote upload tmp dir, local disk cache parent dir). - Drop mkdir/createNewFile calls whose target is created by the following copy or write anyway, and the dead META-INF directory. - Write the R8 main dex list in one call instead of createNewFile plus per-line appends. - Return null from PodSpecParser.getStringListValues for non-string, non-array values instead of dereferencing a null list. - Use Files.setLastModifiedTime in the client test helper so a failure cannot silently leave a stale timestamp. - Remove null guards that sat after an unconditional dereference.
- Strings.CS.removeEnd instead of the deprecated StringUtils.removeEnd. - Share one ContextSnapshotFactory for the context-propagating executors instead of the deprecated ContextSnapshot::captureAll overloads. - Configure the GCP InstancesClient with setBackgroundExecutorProvider and an explicit HTTP/JSON transport executor; setExecutorProvider is deprecated but used to set both, so this keeps the thread cap intact. - Add missing @OverRide annotations in ResolvedPods, rename a shadowing local, drop an unused string copy, an unused test array and unused parameters.
Summary - Extender code coverage reportSummary
Coveragecom/defold/extender - 43.2%
com/defold/extender/builders - 0%
com/defold/extender/cache - 32.8%
com/defold/extender/cache/info - 100%
com/defold/extender/jetty - 82.6%
com/defold/extender/log - 40%
com/defold/extender/metrics - 37.5%
com/defold/extender/process - 73.4%
com/defold/extender/progress - 90.6%
com/defold/extender/remote - 88.9%
com/defold/extender/services - 57.7%
com/defold/extender/services/cocoapods - 52%
com/defold/extender/services/data - 80.7%
com/defold/extender/services/spm - 40.6%
com/defold/extender/tracing - 21.2%
com/defold/extender/utils - 18.1%
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18bd0185aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 459b8305ff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
CommandLineTokenizer treats a backslash as an escape, so a manifest value ending in one swallows the separator space the template emits and merges the next token into it. With the web linkCmd, ["EXPORT_NAME=x\\", "-o/tmp/x.js"] renders as `-s EXPORT_NAME=x\ -s -o/tmp/x.js`, which tokenizes with the attacker's -o as a standalone argv element controlling the output path. Quotes stay allowed: they can only merge tokens, never create one, and the SDK build.yml uses EXPORTED_RUNTIME_METHODS=["ccall", ...].
paths-ignore has no effect for a compiled language analysed with a traced build, so the autobuild step kept extracting server/src/test and alerting on test code. Compile the main source sets explicitly instead; `classes` covers :server, :client and :manifestmergetool without compileTestJava. Keeping paths-ignore as-is, annotated, in case the scan ever moves to buildless analysis.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04615c1d44
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c67fb7fa47
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c55b1e11f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Code fixes
Bugs
AsyncBuilderresult dir is created withFiles.createDirectories; previously a failedmkdirleft no error file and clients polled forever (Add sha256 check for downloaded defoldsdk #334).PodSpecParser.getStringListValuesno longer dereferences null for non-string, non-array values (Added support for users to specify dynamicLibs #108).mkdir/createNewFileresults either converted toFiles.createDirectoriesor removed where the next call creates the target anyway.Deprecations / cleanup
Strings.CS.removeEnd, sharedContextSnapshotFactory, GCPInstancesSettingswith explicit background and transport executors, missing@Override, shadowed local, unused parameters and test array.