Skip to content

Rust: Improve models for environment sources, expect and unwrap#18605

Merged
geoffw0 merged 9 commits intogithub:mainfrom
geoffw0:expect
Jan 29, 2025
Merged

Rust: Improve models for environment sources, expect and unwrap#18605
geoffw0 merged 9 commits intogithub:mainfrom
geoffw0:expect

Conversation

@geoffw0
Copy link
Copy Markdown
Contributor

@geoffw0 geoffw0 commented Jan 28, 2025

Modelling work aimed at getting more results on tests.

  • make existing environment sources more accurate by modelling that they return a Result or Option.
  • add models for .expect and more variants of .unwrap.
  • allow implicit reads at taint sinks in the tests, since some of the existing test cases are written with that assumption.

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code labels Jan 28, 2025
Copilot AI review requested due to automatic review settings January 28, 2025 08:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (5)

rust/ql/test/library-tests/dataflow/local/main.rs:241

  • This new test code is missing a hasValueFlow check, indicating incomplete coverage for the value flow from 48. Please add or update the test annotation.
    sink(s2.unwrap_or_else(|| source(48))); // $ MISSING: hasValueFlow=48

rust/ql/test/library-tests/dataflow/local/main.rs:268

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s1.expect("")); // $ hasValueFlow=78

rust/ql/test/library-tests/dataflow/local/main.rs:269

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s1.expect_err(""));

rust/ql/test/library-tests/dataflow/local/main.rs:272

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s2.expect(""));

rust/ql/test/library-tests/dataflow/local/main.rs:273

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s2.expect_err("")); // $ hasValueFlow=79

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

Comment thread rust/ql/test/library-tests/dataflow/local/main.rs Outdated
@geoffw0
Copy link
Copy Markdown
Contributor Author

geoffw0 commented Jan 28, 2025

The DCA run was uneventful.

Comment thread rust/ql/test/query-tests/security/CWE-312/CleartextLogging.expected Outdated
Comment thread rust/ql/test/library-tests/dataflow/local/main.rs Outdated
@geoffw0
Copy link
Copy Markdown
Contributor Author

geoffw0 commented Jan 28, 2025

Second DCA run also LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants