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

doc: clarify triage comitments #45267

Merged
merged 1 commit into from Nov 8, 2022
Merged

Conversation

mhdawson
Copy link
Member

@mhdawson mhdawson commented Nov 1, 2022

No description provided.

Signed-off-by: Michael Dawson <mdawson@devrus.com>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/node-api

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Nov 1, 2022
Comment on lines +7 to +11
Normally your report will be acknowledged within 5 days, and you'll receive
a more detailed response to your report within 10 days indicating the
next steps in handling your submission. These timelines may extend when
our triage volunteers are away on holiday, particularly at the end of the
year.
Copy link
Member

Choose a reason for hiding this comment

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

"are away on holiday" is...basically..."whenever we say we're not available'....This makes the whole response-time guarantee far less meaningful.

Looking at Python's security document, Chromium's, Ruby's... none of them seem to supply a specific response-time guarantee. I think we should just remove the guarantee entirely or replace it with a vague "will respond promptly" or "will respond in a reasonable time frame".

Copy link
Member

Choose a reason for hiding this comment

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

SGTM to remove the time commitments altogether.

@gireeshpunathil
Copy link
Member

my view: the new commitment text reflects the real posible reasons behind a potential delay in response, aligning to what an open source project could commit at the best.

@Trott
Copy link
Member

Trott commented Nov 2, 2022

@nodejs/security-triage

@mhdawson
Copy link
Member Author

mhdawson commented Nov 2, 2022

I personally prefer setting some kind of expectation so that people know that it won't be immediate and with the addition it's clear that there will also be times when we can't meet the 5/10 days.

@jasnell jasnell changed the title Triage comitments doc: clarify triage comitments Nov 2, 2022
@Trott
Copy link
Member

Trott commented Nov 2, 2022

I personally prefer setting some kind of expectation so that people know that it won't be immediate and with the addition it's clear that there will also be times when we can't meet the 5/10 days.

I think a better approach is an auto-reply to submitted reports with that information. "Thank you for submitting your report. We aim to reply with an initial assessment within 5 days excluding weekends and holidays." That gets the information to the specific person that needs it at the specific time when they need it. That's better, IMO, then adding it to a document they are likely to not read or even know about at all.

@Trott
Copy link
Member

Trott commented Nov 2, 2022

I personally prefer setting some kind of expectation so that people know that it won't be immediate and with the addition it's clear that there will also be times when we can't meet the 5/10 days.

I think a better approach is an auto-reply to submitted reports with that information. "Thank you for submitting your report. We aim to reply with an initial assessment within 5 days excluding weekends and holidays." That gets the information to the specific person that needs it at the specific time when they need it. That's better, IMO, then adding it to a document they are likely to not read or even know about at all.

(Although to be clear: I'm not blocking this. This is entirely up to the security triage team as far as I'm concerned, and you are on the security triage team and I am not. So, it's your call.)

@mhdawson
Copy link
Member Author

mhdawson commented Nov 3, 2022

So I agree an auto respond makes sense in addition to this landing. Don't know though if we have any autoresponders but would be happy if somebody wanted to set one up for this. I do see that as a separate effort though and so it should not block the PR itself.

@mhdawson
Copy link
Member Author

mhdawson commented Nov 3, 2022

@nodejs/security-triage it would be good to hear from more members

@mhdawson
Copy link
Member Author

mhdawson commented Nov 8, 2022

@mcollina you ok with this landing?

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Trott Trott merged commit 9dcfb9e into nodejs:main Nov 8, 2022
Trott added a commit that referenced this pull request Nov 8, 2022
Trott pushed a commit that referenced this pull request Nov 8, 2022
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@Trott
Copy link
Member

Trott commented Nov 8, 2022

Landed in da44fd8

lucshi pushed a commit to lucshi/node that referenced this pull request Nov 9, 2022
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#45267
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Nov 10, 2022
@Trott
Copy link
Member

Trott commented Nov 11, 2022

When I landed this, I forgot to also update https://hackerone.com/nodejs/policy but now I've done that too.

danielleadams pushed a commit that referenced this pull request Dec 30, 2022
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants