-
-
Notifications
You must be signed in to change notification settings - Fork 2k
docs: add security.txt #1974
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
base: gh-pages
Are you sure you want to change the base?
docs: add security.txt #1974
Conversation
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
ctcpip
left a comment
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.
shouldn't we try to comply with the spec?
|
If any reporter requests PGP encryption, we can accommodate them using our personal PGP keys. However, we don’t have a shared/team key at this time.
Personally, I like the idea. It would add an extra step for each report, but many reporters are doing excellent work and I think it’s worth the effort to recognize them publicly. Should we bring this up for discussion in the security working group?
I think English is the best option to simplify report digestion
I am afraid that this is mandatory in the spec (https://www.rfc-editor.org/rfc/rfc9116#name-expires). We don’t expect this information to become stale, but the specification says the Expires field must always be present and recommends that the value be less than a year into the future to avoid staleness. To comply with this requirement, we use one of the following approaches:
We avoid using long-term future dates like the year 2099, since that would technically comply but go against the intent of keeping the file current and accurate. We can do the automation in the future, so we can land this PR soon. |
Instead of setting an expiration date, I'd prefer to define the
Tracking in discussion is a good idea. My personal opinion, we should not do it in open environment. |
@ShubhamOulkar I don't quite understand this idea.
Yes, please bring the discussion to the security team. This decision would be outside the scope of the documentation team.
I can work on this new script, I enjoy automating things. |
|
We should place the "security.txt" file under the "/.well-known/" path, e.g., https://example.com/.well-known/security.txt as per RFC8615 of a domain name. Ref: https://www.rfc-editor.org/rfc/rfc9116#name-location-of-the-securitytxt
Its main aim is to define the process of reporting security vulnerabilities. |
Co-authored-by: shubham oulkar <[email protected]>
Co-authored-by: shubham oulkar <[email protected]>
Co-authored-by: Ulises Gascón <[email protected]>
5709962 to
779c6e5
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
|
I created a script to update the expiration date. For the Acknowledgments field, i created this issue for discussion #2132 |
| # Our security policy | ||
| Policy: https://github.com/expressjs/express/security/policy | ||
| # Expires this policy | ||
| Expires: 2025-12-31T00:00:00Z No newline at end of file |
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.
I left this date to verify that the automation works and opens the PR correctly.
|
cc: @expressjs/security-wg |
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.
I’m generally cautious about automations that open PRs, even though we do already use that pattern in this repo. My first instinct here was a much simpler solutio, a non-blocking CI check validates if the expiry is ~30 days away.
But to review the approach w/ PR creation:
I’d prefer not to add new script infrastructure for something this small. The current approach runs a script which opaquely handles both validation and bumping the date, when really this should be two things:
- do we need to update the expiry soon?
- And then (a human) validating the security.txt and bumping the expires
I don't think what this job is doing is obvious from the GHA, you have to read the script to know what this job is really doing.
We can gate the job and perform the bump entirely inline in GitHub Actions without introducing a bespoke JS parser. For example:
# Gate: is the expiry coming up?
- name: Check security.txt expiry
id: expiry
run: |
expires=$(grep -i '^Expires:' .well-known/security.txt | sed 's/.*: //')
if [ "$(date -d "$expires" +%s)" -le "$(date -d '+30 days' +%s)" ]; then
echo "bump=true" >> "$GITHUB_OUTPUT"
fi
- name: Bump Expires
if: steps.expiry.outputs.bump == 'true'
run: |
sed -i "s/^Expires:.*/Expires: $(date -u -d '+180 days' +%Y-%m-%dT%H:%M:%SZ)/I" \
.well-known/security.txt
- name: Create Pull Request
if: steps.expiry.outputs.bump == 'true'
uses: gr2m/create-or-update-pull-request-action@v1This keeps the automation surface very small and avoids adding one off scripts in the repo. And if something breaks in this, it breaks loudly, and without us having to add any error handling. Which is desireable IMO.
Code is cheap to produce nowadays, but I still strongly prefer producing, reviewing, and maintaining as little as possible.
On intent:
Separately from the mechanics, I want to preserve the spirit of the Expires field. It’s meant to be an explicit revalidation that the contact and disclosure information is still accurate, not just a date that gets mechanically refreshed.
If we do go the PR based route, I’d strongly prefer the PR description explicitly ask reviewers to confirm the contents of security.txt, not just merge to extend the date. This is essential IMO for if none of us are around who understand this file anymore. (requested that change in #1974 (comment))
Co-authored-by: Jon Church <[email protected]> Signed-off-by: Sebastian Beltran <[email protected]>
I have to say I’m not good at Bash, which is why I didn’t do it that way, but your solution seems better and simpler to me. |
break this out into two jobs, * validate if the expiry is coming up in less than 30 days * update expiry w/ sed drop the js script that existed before I like this approach because the logic is broken down into the two tasks, verify the expires needs updating, then update it. With the bonus that all the logic is grokable within the CI itself
|
thanks @jonchurch ! |
| - name: Check security.txt expiry | ||
| id: expiry | ||
| run: | | ||
| set -eo pipefail |
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.
Love me some pipefail
https://gist.github.com/mohanpedala/1e2ff5661761d3abd0385e8223e16425
jonchurch
left a comment
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.
I pushed an update that swaps this to using bash, sed, and grep to accomplish the expires check and then update expires if needed
ref: #1973
preview: https://deploy-preview-1974--expressjscom-preview.netlify.app/.well-known/security.txt
cc: @expressjs/security-wg