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

Update Tor safety instructions on Source Interface per Tor Browser UI changes #4494

Merged
merged 6 commits into from Jun 6, 2019

Conversation

drewmassey
Copy link
Contributor

@drewmassey drewmassey commented May 31, 2019

Status

Ready for Review

Description of Changes

Fixes #4465 .

Changes proposed in this pull request:

  • Deals with some consequences of design changes to Tor 8.5, as outlined by @ninavizz.

Testing

How should the reviewer test this PR?

  • Confirm that the local app and docs are up to date

Deployment

Any special considerations for deployment? Consider both:

  1. Upgrading existing production instances.
  2. New installs.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make -C securedrop test) pass in the development container

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

If you made changes to documentation:

  • Doc linting (make docs-lint) passed locally

@eloquence eloquence changed the title Issue 4465 Update Tor safety instructions on Source Interface per Tor Browser UI changes May 31, 2019
@codecov-io
Copy link

codecov-io commented May 31, 2019

Codecov Report

Merging #4494 into develop will decrease coverage by 0.58%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4494      +/-   ##
===========================================
- Coverage    83.21%   82.63%   -0.59%     
===========================================
  Files           45       45              
  Lines         3069     3115      +46     
  Branches       332      337       +5     
===========================================
+ Hits          2554     2574      +20     
- Misses         430      456      +26     
  Partials        85       85
Impacted Files Coverage Δ
securedrop/securedrop/i18n_tool.py 84.18% <0%> (-10%) ⬇️
securedrop/securedrop/models.py 89.22% <0%> (ø) ⬆️
securedrop/securedrop/source_app/__init__.py 92.63% <0%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52ae022...eabb86a. Read the comment docs.

@zenmonkeykstop
Copy link
Contributor

The functional tests run against Tor Browser 8.0.x, which might be a factor. I'll try running them locally and report back.

@rmol
Copy link
Contributor

rmol commented Jun 1, 2019

I ran them locally earlier without error. 🤷‍♂️

But I should probably update the Dockerfiles to 8.5 next week.

@drewmassey
Copy link
Contributor Author

This is giving a “no such container” issue in circleci but otherwise seems to pass. Is this a known testing issue, such as a race condition? I don’t have permissions to rerun the workflow.

@zenmonkeykstop
Copy link
Contributor

Passed locally for me as well, gave the failing test a kick.

@drewmassey
Copy link
Contributor Author

drewmassey commented Jun 3, 2019

@zenmonkeykstop It looks like your run passed; I wonder if the issue is that the run tests step in circleCI should stop gracefully if it doesn't locate the container? It looks like it complains about not finding the container and then tries to run it anyway, resulting in that step of the pipeline hanging after 10 minutes with no output.

https://circleci.com/gh/freedomofpress/securedrop/28864#tests/containers/1

I haven't fully wrapped my mind around the securedrop CircleCI setup just yet but I have seen this issue of containers not reliably spinning up in circleCI before, resulting in spooky intermittent test failures. My first guess is that I don't exactly understand why the docker container is explicitly deleted here:

https://github.com/freedomofpress/securedrop/blob/develop/.circleci/config.yml#L159

Let me know what you'd like to do; I'd prefer to close out this PR and open a new one for the CI/CD question but we can wait to resolve this as part of this PR if that's preferable to keep confidence on the CircleCI workflow high.

@redshiftzero
Copy link
Contributor

the build failure you're talking about looks like it's CI issue #4480 (if you have an idea for resolution worth doing in a separate PR)

@drewmassey
Copy link
Contributor Author

Yeah I saw that other issue, my guess is the docker rm command I mention above might be fishy, git blame says @redshiftzero put it in so I don’t know if you have a reason for it I’m not thinking of.

Can we resolve this present PR since this issue is documented elsewhere and I will take a deeper dive on #4480 after I tackle #4280 as I promised Nina yesterday? I try to work on exactly one issue at a time when I’m an OSS interloper so as not to demand too much on the maintainers.

@ninavizz
Copy link
Member

ninavizz commented Jun 4, 2019

@drewmassey That issue was just collecting dust; I trust it to get resolved far more quickly as you're able to, than it would otherwise—really, no rush! :) It's a far bigger/broader thing the team has been working on for months/years, getting the infrastructure and dev workflow in a good place. I suspect a set of fresh eyes and a fresh brain, could do a lot of good to that cause, too.

securedrop/source_templates/index.html Show resolved Hide resolved
docs/source.rst Outdated Show resolved Hide resolved
@zenmonkeykstop zenmonkeykstop merged commit 2d066eb into freedomofpress:develop Jun 6, 2019
@zenmonkeykstop zenmonkeykstop mentioned this pull request Jun 27, 2019
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update purple Tor Safest bar on Source Interface per Tor Browser new UX & urgent usability needs
6 participants