Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reporting showerror #109

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/linting/extended_checks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
!is_there_any_star_marker && return x == y

contains(x, "QQQ") && contains(y, "QQQ") &&
throw(BothCannotHaveStarException("Cannot both $x and $y have a star marker"))

Check failure on line 58 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

if contains(x, "QQQ")
reg_exp = Regex(replace(x, "QQQ" => ".*"))
return !isnothing(match(reg_exp, y))
Expand Down Expand Up @@ -219,6 +219,8 @@
struct UseOfStaticThreads <: ViolationLintRule end
struct LogStatementsMustBeSafe <: FatalLintRule end

struct ShowErrorReporting <: RecommendationLintRule end

const all_extended_rule_types = Ref{Any}(
vcat(
InteractiveUtils.subtypes(RecommendationLintRule),
Expand Down Expand Up @@ -290,7 +292,7 @@

function check(t::InitializingWithFunctionRule, x::EXPR, markers::Dict{Symbol,String})
# If we are not in a const statement, then we exit this function.
haskey(markers, :const) || return

Check failure on line 295 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`.
generic_check(t, x, "Threads.nthreads()", "`Threads.nthreads()` should not be used in a constant variable.")
generic_check(t, x, "is_local_deployment()", "`is_local_deployment()` should not be used in a constant variable.")
generic_check(t, x, "Deployment.is_local_deployment()", "`Deployment.is_local_deployment()` should not be used in a constant variable.")
Expand Down Expand Up @@ -346,11 +348,11 @@
check(t::PtrRule, x::EXPR) = generic_check(t, x, "Ptr{hole_variable}(hole_variable)")

function check(t::ArrayWithNoTypeRule, x::EXPR, markers::Dict{Symbol,String})
haskey(markers, :filename) || return

Check failure on line 351 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`.
contains(markers[:filename], "src/Compiler") || return

haskey(markers, :macrocall) && markers[:macrocall] == "@match" && return

Check failure on line 354 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`.
haskey(markers, :macrocall) && markers[:macrocall] == "@matchrule" && return

Check failure on line 355 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`.

generic_check(t, x, "[]", "Need a specific Array type to be provided.")
end
Expand Down Expand Up @@ -390,7 +392,7 @@
end

function check(t::UnsafeRule, x::EXPR, markers::Dict{Symbol,String})
haskey(markers, :function) || return

Check failure on line 395 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`.
isnothing(match(r"_unsafe_.*", markers[:function])) || return
isnothing(match(r"unsafe_.*", markers[:function])) || return

Expand Down Expand Up @@ -486,7 +488,7 @@
end

function check(t::RelPathAPIUsageRule, x::EXPR, markers::Dict{Symbol,String})
haskey(markers, :filename) || return

Check failure on line 491 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`.
contains(markers[:filename], "src/Compiler/Front") || return

generic_check(t, x, "hole_variable::RelPath", "Usage of type `RelPath` is not allowed in this context.")
Expand All @@ -498,7 +500,7 @@
end

function check(t::NonFrontShapeAPIUsageRule, x::EXPR, markers::Dict{Symbol,String})
haskey(markers, :filename) || return

Check failure on line 503 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`.
# In the front-end and in FFI, we are allowed to refer to `Shape`
contains(markers[:filename], "src/FrontCompiler") && return
contains(markers[:filename], "src/FFI") && return
Expand Down Expand Up @@ -533,7 +535,7 @@
is_safe_macro_call(y) =
y.head == :macrocall && y.args[1].head == :IDENTIFIER && y.args[1].val == "@safe"

is_safe_literal(x) = x.head in [:NOTHING,

Check failure on line 538 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `tin(item,collection)` instead of the Julia's `in` or `∈`.
:INTEGER,
:FLOAT,
:TRUE,
Expand Down Expand Up @@ -585,3 +587,8 @@
end
end

function check(t::ShowErrorReporting, x::EXPR)
msg = "Reporting with `showerror(...)` instead of `safe_showerror(...)` could leak sensitive data."
# generic_check(t, x, "showerror(hole_variable_star)", msg)
generic_check(t, x, "showerror", msg)
end
19 changes: 17 additions & 2 deletions test/rai_rules_tests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
result = String(take!(io))
all_lines = split(result, "\n")

verbose && @info result

Check failure on line 31 in test/rai_rules_tests.jl

View workflow job for this annotation

GitHub Actions / run_lint

Unsafe logging statement. You must enclose variables and strings with `@safe(...)`.
# We remove decorations
return length(filter(l->startswith(l, "Line "), all_lines))
end
Expand Down Expand Up @@ -1959,7 +1959,7 @@
precompile_statement=@safe(repr(statement)),
# Log the message that the exception would print, else JSONLogger logs each of
# the fields of the exception separately which is much less useful.
exception=@safe(sprint(showerror, e)),
exception=@safe(sprint(show, e)),
maxlog=100,
)
end
Expand Down Expand Up @@ -2027,4 +2027,19 @@
end
@test result_matching
end
end
end

@testset "showerror reporting" begin
source = """
function rusage()
showerror("an error")
map(showerror, ["a", "b"]);
safe_showerror("an error")
end
"""
@test count_lint_errors(source) == 2
@test lint_test(source,
"Line 2, column 5: Reporting with `showerror(...)` instead of `safe_showerror(...)` could leak sensitive data.")
@test lint_test(source,
"Line 3, column 9: Reporting with `showerror(...)` instead of `safe_showerror(...)` could leak sensitive data.")
end
Loading