Skip to content

Conversation

@tjarratt
Copy link
Contributor

@tjarratt tjarratt commented Oct 30, 2025

Hello !

Similar to #156 I ran into the issue where my Phoenix application has some controllers that are receiving API requests that use an Authorization header. Similar to how the ErrorTracker.Integrations.Plug drops the cookie header, as it is considered sensitive, I strongly believe that we should avoid storing and showing the Authorization header in clear text, as this could allow for an attacker in a Privilege Escalation attack.

For now all I've done is the following

  • backfill tests for the ErrorTracker.Integrations.Plug
  • change the ErrorTracker.Integrations.Plug to avoid logging the Authorization and Cookie headers

Given that there wasn't any existing tests for this module, I'm open to feedback to make these tests more sensible to other users of this codebase.

I'd also like to consider, in a follow-up PR, to obfuscate, rather than avoid logging. In the context for an error occurrence, we would see something like the following

"request_headers" => %{
  "safe_header" => "seen in clear text",
  "cookie" => "<redacted>",
  "authorization" => "<redacted>"
}

Fixes #156
Fixes #150

@tjarratt
Copy link
Contributor Author

tjarratt commented Oct 30, 2025

Mmh, tests are failing on CI. I'll have a look.

edit : it is the test suite failing for Mysql / Maria DB for all elixir versions. I don't understand this error, but I'll have a look with a local maria db.

I believe this to be an existing bug that this PR has uncovered.

     ** (ArgumentError) cannot load `"[]"` as type {:array, :string} for field :breadcrumbs in %ErrorTracker.Occurrence{__meta__: #Ecto.Schema.Metadata<:loaded, "error_tracker_occurrences">, id: nil, reason: nil, context: nil, breadcrumbs: nil, stacktrace: nil, error_id: nil, error: #Ecto.Association.NotLoaded<association :error is not loaded>, inserted_at: nil}
     code: [occurrence] = repo().all(ErrorTracker.Occurrence)
     stacktrace:
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:431: Ecto.Repo.Queryable.struct_load!/6
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:246: anonymous fn/5 in Ecto.Repo.Queryable.preprocessor/3
       (elixir 1.18.4) lib/enum.ex:1714: Enum."-map/2-lists^map/1-1-"/2
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:237: Ecto.Repo.Queryable.execute/4
       (ecto 3.11.2) lib/ecto/repo/queryable.ex:19: Ecto.Repo.Queryable.all/3
       test/integrations/plug_test.exs:48: (test)

@tjarratt
Copy link
Contributor Author

tjarratt commented Oct 30, 2025

I'm seeing that arrays aren't supported on Mysql / Mariadb (even on some issues for Ecto, see this response from Jose Valim back in 2016). This makes me believe that perhaps the breadcrumb feature.

This seems to be blocked by #150. I could implement an ExUnit tag that avoids running these tests with MariaDB for now -- would that be acceptable ?

Okay, after reading a bit more closely, I'm seeing that this now works with ecto_sql from their 3.13.0 release onwards. Adding that as a dependency does get the tests to pass, so I'm inclined to add that to this pull request so that this PR is mergeable, and potentially to close out #150 as well.

Co-authored-by: Chiara Sarta <chiara.sarta@maersk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Field Occurrence.breadcrumbs is broken for MariaDB

1 participant