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

fix: backport xss and rce fixes to v7.1 #834

Merged
merged 3 commits into from
Feb 12, 2024
Merged
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
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
Loading