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

Redact short and known private tokens #692

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

splattael
Copy link
Contributor

@splattael splattael commented Jul 4, 2024

Prior this commit very short private tokens (< 4 chars) triggered: ArgumentError: negative argument

Also, if private token was a term which was part of the inspected string only the first occurrence was redacted.

Refs

https://gitlab.com/gitlab-org/quality/triage-ops/-/merge_requests/2919#note_1983270817

Verification

# Rollback production changes
git checkout master -- lib

bundle exec rspec spec/gitlab/client_spec.rb

Randomized with seed 22140

Gitlab::Client
  #inspect
    with a known private token
      is expected to include "@endpoint=" (FAILED - 1)
      is expected to include "@private_token=\"****oint\"" (FAILED - 2)
    with very short private token
      example at ./spec/gitlab/client_spec.rb:33 (FAILED - 3)
    with a private token
      is expected to include "@private_token=\"******oken\""
    with short private token
      is expected to include "@private_token=\"****\"" (FAILED - 4)
    without private token
      is expected not to include "@private_token="

Failures:

  1) Gitlab::Client#inspect with a known private token is expected to include "@endpoint="
     Failure/Error: it { is_expected.to include('@endpoint=') }
       expected "#<Gitlab::Client:0x00007f7d05a9ff90 @****oint=\"https://api.example.com\", @private_token=\"endpoint\", @user_agent=\"Gitlab Ruby Gem 5.0.0\">" to include "@endpoint="
     # ./spec/gitlab/client_spec.rb:27:in `block (4 levels) in <top (required)>'

  2) Gitlab::Client#inspect with a known private token is expected to include "@private_token=\"****oint\""
     Failure/Error: it { is_expected.to include('@private_token="****oint"') }
       expected "#<Gitlab::Client:0x00007f7d05fc9bd8 @****oint=\"https://api.example.com\", @private_token=\"endpoint\", @user_agent=\"Gitlab Ruby Gem 5.0.0\">" to include "@private_token=\"****oint\""
     # ./spec/gitlab/client_spec.rb:26:in `block (4 levels) in <top (required)>'

  3) Gitlab::Client#inspect with very short private token
     Failure/Error: "#{'*' * (token.size - 4)}#{token[-4..]}"

     ArgumentError:
       negative argument
     # ./lib/gitlab/client.rb:95:in `*'
     # ./lib/gitlab/client.rb:95:in `only_show_last_four_chars'
     # ./lib/gitlab/client.rb:80:in `inspect'
     # ./spec/gitlab/client_spec.rb:9:in `block (3 levels) in <top (required)>'
     # ./spec/gitlab/client_spec.rb:33:in `block (4 levels) in <top (required)>'

  4) Gitlab::Client#inspect with short private token is expected to include "@private_token=\"****\""
     Failure/Error: it { is_expected.to include('@private_token="****"') }
       expected "#<Gitlab::Client:0x00007f7d05cb9ba0 @endpoint=\"https://api.example.com\", @private_token=\"abcd\", @user_agent=\"Gitlab Ruby Gem 5.0.0\">" to include "@private_token=\"****\""
     # ./spec/gitlab/client_spec.rb:39:in `block (4 levels) in <top (required)>'

Finished in 0.0212 seconds (files took 0.19272 seconds to load)
6 examples, 4 failures

Failed examples:

rspec ./spec/gitlab/client_spec.rb:27 # Gitlab::Client#inspect with a known private token is expected to include "@endpoint="
rspec ./spec/gitlab/client_spec.rb:26 # Gitlab::Client#inspect with a known private token is expected to include "@private_token=\"****oint\""
rspec ./spec/gitlab/client_spec.rb:33 # Gitlab::Client#inspect with very short private token
rspec ./spec/gitlab/client_spec.rb:39 # Gitlab::Client#inspect with short private token is expected to include "@private_token=\"****\""

Randomized with seed 22140

@splattael splattael force-pushed the splattael/short-token branch from 41bfc86 to f584861 Compare July 4, 2024 15:28
@splattael
Copy link
Contributor Author

👋 @NARKOZ If time allows, do you mind reviewing this pull request? 🙏

@splattael splattael force-pushed the splattael/short-token branch from f584861 to 0b6a132 Compare July 4, 2024 15:32
Prior this commit very short private tokens (< 4 chars) triggered:
ArgumentError: negative argument

Also, if private token was a term which was part of the inspected string
only the first occurrence was redacted.
@splattael splattael force-pushed the splattael/short-token branch from 0b6a132 to 1024223 Compare July 4, 2024 15:33
@NARKOZ NARKOZ merged commit 227a47c into NARKOZ:master Jul 8, 2024
4 checks passed
@NARKOZ
Copy link
Owner

NARKOZ commented Jul 8, 2024

Thank you ❤️

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.

2 participants