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

core: commander: Add environment_variables endpoint #1467

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

patrickelectric
Copy link
Member

@patrickelectric patrickelectric commented Feb 13, 2023

Helps #1466

image

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
@Williangalvani
Copy link
Member

I wonder if these could be used to expose things we dont want exposed, such as CI variables

@patrickelectric
Copy link
Member Author

I wonder if these could be used to expose things we dont want exposed, such as CI variables

You already have access to it via the terminal

@joaoantoniocardoso
Copy link
Collaborator

joaoantoniocardoso commented Feb 15, 2023

I wonder if these could be used to expose things we dont want exposed, such as CI variables

You already have access to it via the terminal

I believe the env variables set for creating the docker (like docker hub keys) wouldn't be accessible from inside the docker if we don't deliberately pass them with ENV or as args.

@ES-Alexander
Copy link
Collaborator

ES-Alexander commented Feb 15, 2023

I believe the [potentially important stuff] wouldn't be accessible from inside the docker ...

I may be misunderstanding this, but given we can run red-pill from the terminal in the docker to access the base operating system, everything outside the docker is technically already accessible from inside the docker is it not? At least I imagine that's the case from a privacy/security standpoint, except for things that require elevating privileges.

@joaoantoniocardoso
Copy link
Collaborator

@ES-Alexander

  • Inside the docker:
    image

  • Outside the docker:
    image

@ES-Alexander
Copy link
Collaborator

  • Inside the docker:
    ...
  • Outside the docker:
    ...

Yes, but my point was that we have a tool inside the docker that allows you to access the outside, so from a security standpoint anything we can access via the web terminal that doesn't require putting in a password is also accessible to an arbitrary program running in the docker.

@joaoantoniocardoso
Copy link
Collaborator

joaoantoniocardoso commented Feb 15, 2023

  • Inside the docker:
    ...
  • Outside the docker:
    ...

Yes, but my point was that we have a tool inside the docker that allows you to access the outside, so from a security standpoint anything we can access via the web terminal that doesn't require putting in a password is also accessible to an arbitrary program running in the docker.

I agree, and that security hole has existed since we added the commander (/command/host). I think the commander should have a restrictive CORS and only allows access from the same address, but this discussion has no relation with environment variables or this PR.

@Williangalvani
Copy link
Member

I was actually thinking of CI, where our dockerhub credentials were avaialable, but github does require us to allow CI to run for new contributors, and also I think now that is only provided to the deployment action

@patrickelectric patrickelectric merged commit 8804129 into bluerobotics:master Feb 15, 2023
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.

None yet

4 participants