GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: clarify triage comitments by mhdawson · Pull Request #45267 · nodejs/node

Conversation

@mhdawson

@mhdawson mhdawson commented Nov 1, 2022

Copy link
Copy Markdown
Member

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mhdawson
doc: allow for holidays in triage response
05dbd2d 
Signed-off-by: Michael Dawson <[email protected]>
@nodejs-github-bot

nodejs-github-bot commented Nov 1, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/node-api
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Nov 1, 2022
@mhdawson
mhdawson force-pushed the triage-comitments branch from 9051f1a to 05dbd2d Compare November 1, 2022 16:18
Trott
Trott reviewed Nov 1, 2022
View reviewed changes
Comment thread SECURITY.md
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.

@Trott Trott Nov 1, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RafaelGSS, anonrig, RaisinTen, mcollina, legendecas, and jasnell reacted with thumbs up emoji
All reactions
  • 👍 6 reactions

@mcollina mcollina Nov 2, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gireeshpunathil

gireeshpunathil commented Nov 2, 2022

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gireeshpunathil
gireeshpunathil approved these changes Nov 2, 2022
View reviewed changes
@Trott

Trott commented Nov 2, 2022

Copy link
Copy Markdown
Member

@nodejs/security-triage
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhdawson

mhdawson commented Nov 2, 2022

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell changed the title Triage comitments doc: clarify triage comitments Nov 2, 2022
@Trott

Trott commented Nov 2, 2022

Copy link
Copy Markdown
Member

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott

Trott commented Nov 2, 2022

Copy link
Copy Markdown
Member

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.)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhdawson

mhdawson commented Nov 3, 2022

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhdawson

mhdawson commented Nov 3, 2022

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhdawson

mhdawson commented Nov 8, 2022

Copy link
Copy Markdown
Member Author

@mcollina you ok with this landing?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mcollina
mcollina approved these changes Nov 8, 2022
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Trott
Trott merged this pull request into nodejs:main Nov 8, 2022
Trott pushed a commit that referenced this pull request Nov 8, 2022
@mhdawson @Trott
doc: allow for holidays in triage response
da44fd8 
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@Trott

Trott commented Nov 8, 2022

Copy link
Copy Markdown
Member

Landed in da44fd8
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
@mhdawson @RafaelGSS
doc: allow for holidays in triage response
0e9bad9 
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Nov 10, 2022
v19.1.0 proposal #45269
Merged
@Trott

Trott commented Nov 11, 2022

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
v18.13.0 release proposal #46025
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@mhdawson @danielleadams
doc: allow for holidays in triage response
6749687 
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@mhdawson @danielleadams
doc: allow for holidays in triage response
00e02bd 
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45267
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott left review comments
@mcollina mcollina mcollina approved these changes
@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@mhdawson @nodejs-github-bot @gireeshpunathil @Trott @mcollina
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.