-
Notifications
You must be signed in to change notification settings - Fork 121
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
Initial project boilerplate and README #9
Initial project boilerplate and README #9
Conversation
Please do not merge this until the Charter is approved. |
@williamkapke Done, I was a bit literal with the XXX date, because I don't know what date that is supposed to be. Do you have a ref to charter approval process? I thought the TSC had requested the group be formed. Any suggestions you have for following the formal process would be welcome. |
@@ -1,9 +1,27 @@ | |||
# Node.js Security Working Group | |||
# Security Work Group |
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.
Nit: Working Group
is what all the other WGs are listed as, so should probably keep that consistent here.
Still not a fan of the name. There's already a security team that can be @-mentioned as nodejs/security. This is bound to be a cause of tons of confusion. I'd prefer |
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 with change of name to "Security Policy Working Group"
@williamkapke or anyone... what is the process for incorporating a working group? I assume that process would allow some discussion of the name. I agree, "Security Working Group" as a name is confusing because there is already a security team/email address, but I don't like the "Policy" variant either, because some of the things discussed that the WG could do are not policy related. I think the security team should be renamed, since its purpose is narrow enough to easily name, and its camping on a generic term. It would make sense to call it the vulnerability response team or something of the like, and then this WG/team could be the Security WG without confusion. |
For others that come along- check out: nodejs/TSC#175 (comment) In short- I think we might be talking about a "Node.js Security Top-Level Project" and a "Security Technical Committee" instead. |
Merged all content from #1 |
As far as mandate goes, and following on from #1, I think the README shouldn't be as specific as it is. The mandate is specifically to be custodian of node security related work, and generally to improve security of Node.js and its ecosystem. We could leave it at that (but better worded). Specific things we could do can be brought up as issues, discussed on github or in WG meetings, and then added to the README as necessary and useful, or just left in the issues and meeting notes if that's all that is required. This would allow discussion of, for example, how we could contribute to ecosystem security without attempting to get involved in thirdparty project's security without being invited. |
I think giving people an idea of what we think is currently in scope is not a bad thing. The Mandate can also change so its not like its we can't change our minds later. But given that the group has not yet met to discuss either waiting to land until some discussion takes place could make sense or making it less specific would be ok. However, being as specific as is useful once there has been discussion seems best to me. |
PTAL I need one more LGTM I added everyone from the repository @nodejs/security-wg team to the README as team members, since you have all accepted. If that isn't correct, please tell me. |
@williamkapke does the README address #9 (comment) ? LGTY? |
I'm not as expert on Node process as @williamkapke is, but to my untrained eye, the contents of this PR looks fine, so LGTM from me. |
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
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.
A few nits - but otherwise looking good!
|
||
|
||
Code of Conduct | ||
|
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.
Many repos just add a pointer to the main copy of the CoC:
https://github.com/nodejs/node/blob/master/CODE_OF_CONDUCT.md
@@ -4,8 +4,19 @@ The MIT License (MIT) | |||
Copyright (c) 2016 Node.js Foundation |
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.
This could be bumped to 2017 now...
ref: nodejs/TSC#195
@williamkapke PTAL |
There is no specific set of requirements or qualifications for WG membership | ||
beyond these rules. | ||
|
||
The WG may add additional members to the WG by unanimous consensus. |
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.
Could you include a link to the @nodejs/ team that comprises the members?
Also will there be a team for Collaborators? I assume @nodejs/security-wg
is the members.
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.
There is no link that is viewable, except by members, teams are private. Also, did you notice I listed all the members in the README?
There is no distinction between member and Collaborator in any of the WGs that I am aware of.
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.
@sam-github I agree that I've never heard of a collaborator/member distinction before, which is why I found this confusing:
The security GitHub repository is maintained by the WG and additional Collaborators who are added by the WG on an ongoing basis. Individuals making significant and valuable contributions are made Collaborators and given commit-access to the project.
This implies that the members (the WG
) are a subsection of Collaborators, otherwise why have one paragraph called Collaborators
and one paragraph called Members
?
I noticed that you listed the Collaborators in the README, which is why I asked about the members.
The link is viewable by members of the nodejs org, so by writing @nodejs/security-wg
(and possibly linking to https://github.com/orgs/nodejs/teams/security-wg) you make it clear which team corresponds to the members/collaborators list.
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.
The GOVERNANCE of the security-wg is identical to https://github.com/nodejs/benchmarking/blob/master/GOVERNANCE.md, I don't want to have customized governance policy per working group. The governance documents and policies should all, ultimately, be centralized so that they can be referenced by link. Lets not diverge. Its already odd that some WGs have a governance, and others don't (https://github.com/nodejs/build). I'm tempted to just delete this, it doesn't seem necessary.
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.
It is on my TODO list to start a discussion about the need for a GOVERNANCE.md vs. just putting it in the README (which I'd prefer). There COULD be a reason... I just don't know it. I've just been shooting for consistency for now.
PR-URL: #9 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Adam Brady <[email protected]>
PR-URL: #9 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Adam Brady <[email protected]>
PR-URL: nodejs/security-wg#9 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Adam Brady <[email protected]>
PR-URL: nodejs/security-wg#9 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Adam Brady <[email protected]>
PR-URL: nodejs/security-wg#9 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Adam Brady <[email protected]>
PR-URL: nodejs/security-wg#9 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Adam Brady <[email protected]>
No description provided.