Skip to content

Commit

Permalink
fix: backport xss and rce fixes to v7.1 (#834)
Browse files Browse the repository at this point in the history
* fix: backport xss and rce fixes to v7.1

This was fixed in #829 and #833

* chore(lint): lint'em real good

* chore: remove reek
  • Loading branch information
mhenrixon authored Feb 12, 2024
1 parent 81cc875 commit cd09ba6
Show file tree
Hide file tree
Showing 18 changed files with 78 additions and 85 deletions.
15 changes: 0 additions & 15 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,3 @@ jobs:
bundler-cache: true
- run: bin/bundle --jobs=$(nproc) --retry=$(nproc)
- run: bin/rubocop -P

reek:
runs-on: ubuntu-latest
strategy:
fail-fast: true

steps:
- uses: actions/checkout@v3
- uses: ruby/setup-ruby@v1
with:
ruby-version: 3.1
bundler: 2.4.12
bundler-cache: true
- run: bin/bundle --jobs=$(nproc) --retry=$(nproc)
- run: bin/reek .
7 changes: 7 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ RSpec/RepeatedExample:
Exclude:
- spec/sidekiq_unique_jobs/unique_args_spec.rb

RSpec/SpecFilePathFormat:
Exclude:
- spec/performance/lock_digest_spec.rb
- spec/performance/locksmith_spec.rb
- spec/sidekiq_unique_jobs/configuration_spec.rb
- spec/sidekiq_unique_jobs/middleware/server/until_and_while_executing_spec.rb

Style/Documentation:
Enabled: true
Exclude:
Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/sidekiq_unique_jobs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def configure(options = {})
yield config
else
options.each do |key, val|
config.send("#{key}=", val)
config.send(:"#{key}=", val)
end
end
end
Expand Down
31 changes: 16 additions & 15 deletions lib/sidekiq_unique_jobs/web.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ def self.registered(app) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
end

app.get "/changelogs" do
@filter = params[:filter] || "*"
@filter = h(params[:filter] || "*")
@filter = "*" if @filter == ""
@count = (params[:count] || 100).to_i
@current_cursor = params[:cursor]
@prev_cursor = params[:prev_cursor]
@count = h(params[:count] || 100).to_i
@current_cursor = h(params[:cursor])
@prev_cursor = h(params[:prev_cursor])
@total_size, @next_cursor, @changelogs = changelog.page(
cursor: @current_cursor,
pattern: @filter,
Expand All @@ -34,11 +34,11 @@ def self.registered(app) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
end

app.get "/locks" do
@filter = params[:filter] || "*"
@filter = h(params[:filter] || "*")
@filter = "*" if @filter == ""
@count = (params[:count] || 100).to_i
@current_cursor = params[:cursor]
@prev_cursor = params[:prev_cursor]
@count = h(params[:count] || 100).to_i
@current_cursor = h(params[:cursor])
@prev_cursor = h(params[:prev_cursor])

@total_size, @next_cursor, @locks = digests.page(
cursor: @current_cursor,
Expand All @@ -50,11 +50,11 @@ def self.registered(app) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
end

app.get "/expiring_locks" do
@filter = params[:filter] || "*"
@filter = h(params[:filter] || "*")
@filter = "*" if @filter == ""
@count = (params[:count] || 100).to_i
@current_cursor = params[:cursor]
@prev_cursor = params[:prev_cursor]
@count = h(params[:count] || 100).to_i
@current_cursor = h(params[:cursor])
@prev_cursor = h(params[:prev_cursor])

@total_size, @next_cursor, @locks = expiring_digests.page(
cursor: @current_cursor,
Expand All @@ -72,7 +72,7 @@ def self.registered(app) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
end

app.get "/locks/:digest" do
@digest = params[:digest]
@digest = h(params[:digest])
@lock = SidekiqUniqueJobs::Lock.new(@digest)

erb(unique_template(:lock))
Expand All @@ -85,9 +85,10 @@ def self.registered(app) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
end

app.get "/locks/:digest/jobs/:job_id/delete" do
@digest = params[:digest]
@digest = h(params[:digest])
@job_id = h(params[:job_id])
@lock = SidekiqUniqueJobs::Lock.new(@digest)
@lock.unlock(params[:job_id])
@lock.unlock(@job_id)

redirect_to "locks/#{@lock.key}"
end
Expand Down
2 changes: 1 addition & 1 deletion spec/performance/lock_digest_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

RSpec.describe SidekiqUniqueJobs::LockDigest, perf: true do
RSpec.describe SidekiqUniqueJobs::LockDigest, :perf do
let(:lock_digest) { described_class.new(item) }
let(:job_class) { UntilExecutedJob }
let(:class_name) { worker_class.to_s }
Expand Down
2 changes: 1 addition & 1 deletion spec/performance/locksmith_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

RSpec.describe SidekiqUniqueJobs::Locksmith, perf: true do
RSpec.describe SidekiqUniqueJobs::Locksmith, :perf do
let(:locksmith_one) { described_class.new(item_one) }
let(:locksmith_two) { described_class.new(item_two) }

Expand Down
26 changes: 13 additions & 13 deletions spec/sidekiq_unique_jobs/changelog_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@

it "clears out all entries" do
expect { clear }.to change { entity.entries.size }.by(-1)
expect(clear).to be == 1
expect(clear).to eq 1
end
end

context "without entries" do
it "returns 0 (zero)" do
expect { clear }.not_to change { entity.entries.size }
expect(clear).to be == 0
expect(clear).to eq 0
end
end
end
Expand All @@ -55,27 +55,27 @@
subject(:exist?) { entity.exist? }

context "when no entries exist" do
it { is_expected.to be == false }
it { is_expected.to be false }
end

context "when entries exist" do
before { simulate_lock(key, job_id) }

it { is_expected.to be == true }
it { is_expected.to be true }
end
end

describe "#pttl" do
subject(:pttl) { entity.pttl }

context "when no entries exist" do
it { is_expected.to be == -2 }
it { is_expected.to eq(-2) }
end

context "when entries exist without expiration" do
before { simulate_lock(key, job_id) }

it { is_expected.to be == -1 }
it { is_expected.to eq(-1) }
end

context "when entries exist with expiration" do
Expand All @@ -92,13 +92,13 @@
subject(:ttl) { entity.ttl }

context "when no entries exist" do
it { is_expected.to be == -2 }
it { is_expected.to eq(-2) }
end

context "when entries exist without expiration" do
before { simulate_lock(key, job_id) }

it { is_expected.to be == -1 }
it { is_expected.to eq(-1) }
end

context "when entries exist with expiration" do
Expand All @@ -107,15 +107,15 @@
expire(key.changelog, 600)
end

it { is_expected.to be == 600 }
it { is_expected.to eq 600 }
end
end

describe "#expires?" do
subject(:expires?) { entity.expires? }

context "when no entries exist" do
it { is_expected.to be == false }
it { is_expected.to be false }
end

context "when entries exist" do
Expand All @@ -124,21 +124,21 @@
expire(key.changelog, 600)
end

it { is_expected.to be == true }
it { is_expected.to be true }
end
end

describe "#count" do
subject(:count) { entity.count }

context "when no entries exist" do
it { is_expected.to be == 0 }
it { is_expected.to eq 0 }
end

context "when entries exist" do
before { simulate_lock(key, job_id) }

it { is_expected.to be == 2 }
it { is_expected.to eq 2 }
end
end

Expand Down
10 changes: 5 additions & 5 deletions spec/sidekiq_unique_jobs/cli_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
jobs del PATTERN
Options:
d, [--dry-run], [--no-dry-run] # set to false to perform deletion
c, [--count=N] # The max number of digests to return
# Default: 1000
-d, [--dry-run], [--no-dry-run] # set to false to perform deletion
-c, [--count=N] # The max number of digests to return
# Default: 1000
deletes unique digests from redis by pattern
HEADER
Expand All @@ -55,8 +55,8 @@
jobs list PATTERN
Options:
c, [--count=N] # The max number of digests to return
# Default: 1000
-c, [--count=N] # The max number of digests to return
# Default: 1000
list all unique digests and their expiry time
HEADER
Expand Down
12 changes: 6 additions & 6 deletions spec/sidekiq_unique_jobs/lua/delete_by_digest_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@
it "deletes the expected keys from redis" do
expect(delete_by_digest).to eq(8)

expect(queued.count).to be == 0
expect(primed.count).to be == 0
expect(locked.count).to be == 0
expect(queued.count).to eq 0
expect(primed.count).to eq 0
expect(locked.count).to eq 0

expect(run_queued.count).to be == 0
expect(run_primed.count).to be == 0
expect(run_locked.count).to be == 0
expect(run_queued.count).to eq 0
expect(run_primed.count).to eq 0
expect(run_locked.count).to eq 0
end
end
2 changes: 1 addition & 1 deletion spec/sidekiq_unique_jobs/lua/lock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
let(:now_f) { SidekiqUniqueJobs.now_f }
let(:lock_limit) { 1 }

shared_context "with a primed key", with_primed_key: true do
shared_context "with a primed key", :with_primed_key do
before do
call_script(:queue, key.to_a, argv_one)
rpoplpush(key.queued, key.primed)
Expand Down
12 changes: 6 additions & 6 deletions spec/sidekiq_unique_jobs/lua/unlock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
let(:locked_jid) { job_id_one }
let(:lock_limit) { 1 }

shared_context "with a lock", with_a_lock: true do
shared_context "with a lock", :with_a_lock do
before do
call_script(:queue, key.to_a, argv_one)
rpoplpush(key.queued, key.primed)
Expand Down Expand Up @@ -172,10 +172,10 @@
it "does not unlock" do
expect { unlock }.to change { changelogs.count }.by(1)

expect(queued.count).to be == 0
expect(primed.count).to be == 0
expect(queued.count).to eq 0
expect(primed.count).to eq 0

expect(locked.count).to be == 1
expect(locked.count).to eq 1
expect(locked.entries).to contain_exactly(job_id_two)
expect(locked[job_id_two].to_f).to be_within(0.5).of(now_f)
end
Expand All @@ -189,9 +189,9 @@
expect(queued.count).to eq(1)
expect(queued.entries).to contain_exactly("1")

expect(primed.count).to be == 0
expect(primed.count).to eq 0

expect(locked.count).to be == 0
expect(locked.count).to eq 0
expect(locked.entries).to be_empty
expect(locked[job_id_one]).to be_nil
end
Expand Down
4 changes: 2 additions & 2 deletions spec/sidekiq_unique_jobs/redis/hash_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@
subject(:count) { entity.count }

context "without entries" do
it { is_expected.to be == 0 }
it { is_expected.to eq 0 }
end

context "with entries" do
before { hset(digest, job_id, now_f) }

it { is_expected.to be == 1 }
it { is_expected.to eq 1 }
end
end
end
4 changes: 2 additions & 2 deletions spec/sidekiq_unique_jobs/redis/set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
subject(:count) { entity.count }

context "without entries" do
it { is_expected.to be == 0 }
it { is_expected.to eq 0 }
end

context "with entries" do
before { sadd(digest, job_id) }

it { is_expected.to be == 1 }
it { is_expected.to eq 1 }
end
end
end
Loading

0 comments on commit cd09ba6

Please sign in to comment.