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

Show full URL of retrieved security.txt file instead of just host #1314

Open
janwillemstegink opened this issue Mar 8, 2024 · 3 comments · May be fixed by #1319
Open

Show full URL of retrieved security.txt file instead of just host #1314

janwillemstegink opened this issue Mar 8, 2024 · 3 comments · May be fixed by #1319

Comments

@janwillemstegink
Copy link

The explanation for security.txt may show a URL that you can copy and paste to avoid typing problems.

See an incomplete path for example in:
https://en.internet.nl/site/rijksoverheid.nl/2671942/#control-panel-31

currently: security.txt retrieved from www.ncsc.nl.
proposed: (security.txt retrieved from) https://www.ncsc.nl/.well-known/security.txt

@bwbroersma
Copy link
Collaborator

An extra enhancement would be to see the complete redirect path, but just showing the full resolved path of the sectxt would already indeed be very useful.

bwbroersma added a commit to bwbroersma/Internet.nl that referenced this issue Mar 10, 2024
@bwbroersma bwbroersma linked a pull request Mar 10, 2024 that will close this issue
bwbroersma added a commit to bwbroersma/Internet.nl that referenced this issue Mar 10, 2024
bwbroersma added a commit to bwbroersma/Internet.nl that referenced this issue Mar 10, 2024
@bwbroersma
Copy link
Collaborator

bwbroersma commented Mar 10, 2024

I think I have a patch. Note that is will only update the 'retrieved from', not the 'requested from', since the last one will likely otherwise show /security.txt because the legacy path is tried as a fallback:

if response.status_code != 200:
http_kwargs["path"] = SECURITYTXT_LEGACY_PATH


Some samples of 'Technical details' (of rijksoverheid.nl and example.org):

Web server IP address Findings
178.22.85.9 security.txt retrieved from https://www.ncsc.nl/.well-known/security.txt.
Web server IP address Findings
93.184.216.34 security.txt requested from example.org.
... Error: security.txt could not be located.

Another thing is this would break all old reports, since they don't have the url in the database. I fixed this by filling in the url with the host variable for old reports. So the url will only show in new reports (once this is merged and deployed).

Maybe the dot should be removed behind the url for easier copying? It's still there because I just changed the {hostname} to {url}, without changing the message.

bwbroersma added a commit to bwbroersma/Internet.nl that referenced this issue Mar 10, 2024
@mxsasha
Copy link
Collaborator

mxsasha commented Mar 11, 2024

I think I have a patch. Note that is will only update the 'retrieved from', not the 'requested from', since the last one will likely otherwise show /security.txt because the legacy path is tried as a fallback:

True, though we could preserve the original path. I think this approach is reasonable too.

Another thing is this would break all old reports, since they don't have the url in the database. I fixed this by filling in the url with the host variable for old reports. So the url will only show in new reports (once this is merged and deployed).

Yes, this backfill is a good approach.

Maybe the dot should be removed behind the url for easier copying? It's still there because I just changed the {hostname} to {url}, without changing the message.

We should remove the dot, yes

@mxsasha mxsasha changed the title explanation for security.txt may contain a URL that you can copy and paste Show full URL of retrieved security.txt file instead of just host Mar 11, 2024
mxsasha pushed a commit to bwbroersma/Internet.nl that referenced this issue Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants