Skip to content

Commit

Permalink
Validate page refresh interval to ensure a minimum amount of delay
Browse files Browse the repository at this point in the history
  • Loading branch information
mperham committed Sep 5, 2023
1 parent 20e7ca2 commit 62c90d7
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
1 change: 1 addition & 0 deletions web/assets/javascripts/application.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion web/assets/javascripts/dashboard-charts.js
Expand Up @@ -57,7 +57,9 @@ class DashboardChart extends BaseChart {
class RealtimeChart extends DashboardChart {
constructor(el, options) {
super(el, options);
this.delay = parseInt(localStorage.sidekiqTimeInterval) || 5000;
let d = parseInt(localStorage.sidekiqTimeInterval) || 5000;
if (d < 2000) { d = 2000; }
this.delay = d
this.startPolling();
document.addEventListener("interval:update", this.handleUpdate.bind(this));
}
Expand Down

11 comments on commit 62c90d7

@jbirdjavi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be backported to 6.x?

@mperham
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just cherry picked it into the 6-x branch so it'll be in a future 6.5.10 release. No ETA for that.

@Earlopain
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CVE seems misleading. This is only client-side validation and nothing is stopping me from making these requests myself, the DoS is still present.

I would think/hope that most already have the dashboard behind a rails constraint or something similar like the build-in enterprise authorization feature.

@bmedenwald
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ETA yet on the 6.5.10 release?

@olleicua
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7.x does not seem to be backwards compatible. This seems like it should merit a prompt release for 6.x.

@olleicua
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mperham Is there still no ETA for this?

@soberstadt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone who feels the need for this fix on 6.x, you can pin your Gemfile to the back-ported fix: 101435c

I wouldn't expect a new version to be cut for this specifically. Mike has said that he considers this low severity.

@kflavin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone who feels the need for this fix on 6.x, you can pin your Gemfile to the back-ported fix: 101435c

I wouldn't expect a new version to be cut for this specifically. Mike has said that he considers this low severity.

Agree. But for those of us who use Github Dependabot (or bundler-audit), this getting flagged as high raises questions and awkward conversations with anyone we need to share the reports with (ie: customers).

image

Upgrading to 7 will be a large effort for us.

@olleicua
Copy link

@olleicua olleicua commented on 62c90d7 Oct 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone who feels the need for this fix on 6.x, you can pin your Gemfile to the back-ported fix: 101435c
I wouldn't expect a new version to be cut for this specifically. Mike has said that he considers this low severity.

Agree. But for those of us who use Github Dependabot (or bundler-audit), this getting flagged as high raises questions and awkward conversations with anyone we need to share the reports with (ie: customers).
image

Upgrading to 7 will be a large effort for us.

I agree that it is quite low severity but I have no idea how to tell dependabot that and right now we have dependabot turned off because of this which is dangerous. Perhaps the greater issue here is that v7 is not backwards compatible and the transition guides are incomplete / confusing.

@mperham
Copy link
Collaborator Author

@mperham mperham commented on 62c90d7 Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.5.10 will be out tomorrow (Monday morning Pacific) and include this CVE fix plus a fix for Rails 7.1.

@olleicua
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.5.10 will be out tomorrow (Monday morning Pacific) and include this CVE fix plus a fix for Rails 7.1.

Thank you!!

Please sign in to comment.