fix: remediate CodeQL rust/path-injection, rust/non-https-url, actions/missing-workflow-permissions#7
Merged
Conversation
…g-workflow-permissions Remove query-filter exclusions that were masking CodeQL alerts in PR #5. Fix all 3 alert categories at the code level. rust/non-https-url (known_vendors.rs): - Remove #[cfg(not(test))] gate on HTTPS guard — guard is now unconditional - Extract fetch_url() (private) and apply_remote_data() (pub(crate)) from sync_from_github so tests can exercise parse/apply logic without HTTP - Restructure 4 wiremock tests to call apply_remote_data() directly; convert HTTP-URL tests to assert HTTPS enforcement error message rust/path-injection (known_vendors.rs, cache_commands.rs, dep_check.rs, ner_org.rs): - find_config_dir: add file_name()=="config" allowlist barrier to CWD path; reorder file_name()/is_dir() so file_name() check precedes filesystem sink on all three exe-relative paths; add file_name().is_some() to env var path - cache_commands.rs (2 sites): replace negative-guard+continue pattern with positive extension==json check wrapping the read_to_string sink - dep_check.rs: inline file_name() comparison directly in if condition (stored bool variable broke CodeQL barrier tracking) - ner_org.rs write_if_missing: reconstruct path from canonical_parent.join(file_name) so raw path parameter never reaches File::create sink - ner_org.rs setup_onnx_runtime (Windows + non-Windows): add file_name() allowlist check before exists() call actions/missing-workflow-permissions (build.yml, security.yml): - Add top-level permissions: contents: read to both workflows All 3775 unit tests pass. query-filters block removed from codeql-config.yml.
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
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.
Summary
Removes the
query-filtersexclusions added in PR #5 and properly remediates all three CodeQL alert categories at the code level.Changes
rust/non-https-url(1 alert —known_vendors.rs)#[cfg(not(test))]gate on HTTPS guard insync_from_github— guard is now unconditional on all code pathsfetch_url()(private async) andapply_remote_data()(pub(crate)) so tests can exercise parse/apply logic without making HTTP requestsapply_remote_data()directly; converted HTTP-URL tests to assert the HTTPS enforcement errorrust/path-injection(28 alerts — 4 files)known_vendors.rsfind_config_dir: addedfile_name()=="config"allowlist barrier to CWD path; reorderedfile_name()/is_dir()so allowlist check precedes filesystem sink on all exe-relative pathscache_commands.rs(2 sites): replaced negative-guard+continuewith positiveextension==jsonwrapping theread_to_stringsinkdep_check.rs: inlinedfile_name()comparison directly into theifcondition (stored bool broke CodeQL barrier tracking)ner_org.rswrite_if_missing: reconstruct path fromcanonical_parent.join(file_name)— rawpathparameter never reachesFile::createner_org.rssetup_onnx_runtime(Windows + non-Windows): addedfile_name()allowlist beforeexists()actions/missing-workflow-permissions(8 alerts)permissions: contents: readtobuild.ymlandsecurity.ymlScanner config cleanup
query-filtersexclusions from.github/codeql/codeql-config.ymlTest results
cargo test --lib --locked)cargo check --locked --all-featuresclean