dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.61k stars 979 forks source link

Cargo private registry support #3478

Closed jamesloosli closed 4 months ago

jamesloosli commented 3 years ago

Cargo scanning doesn't seem to support private cargo registries.

It'd be great to be able to add something like this;

registries:
  cargo-example:
    type: cargo-registry
    url: https://dl.cloudsmith.io/basic/OWNER/REPOSITORY/cargo/index.git
    username: cargo-user
    password: ${{secrets.CARGO_GIT_PASSWORD}}
asciimike commented 3 years ago

We've been recommending folks use the git registry, would that work? E.g.:

registries:
  cargo-example:
    type: git
    url: https://dl.cloudsmith.io/basic/OWNER/REPOSITORY/cargo/index.git
    username: cargo-user
    password: ${{secrets.CARGO_GIT_PASSWORD}}

Alternatively, are you looking for https://github.com/dependabot/dependabot-core/issues/1767?

tomershaniii commented 3 years ago

hi @asciimike , When trying the above snippet, i'm receiving the following error: "Git registries must specify a source in a format like https://github.com or https://gitlab.com. Please don't include a path." is this a regression? any alternative syntax we can use?

asciimike commented 3 years ago

@tomershaniii I believe you can just use

registries:
  cargo-example:
    type: git
    url: https://dl.cloudsmith.io/
    username: cargo-user
    password: ${{secrets.CARGO_GIT_PASSWORD}}

As the actual dependency should have the full URL. Mind giving that a try and letting me know if it works?

tomershaniii commented 3 years ago

unfortunately it does not, i've reached out to the clousmith team who indicated the full url is required

ciaracarey commented 3 years ago

Hi Mike, I'm Ciara from Cloudsmith and trying to help Tomer with this issue. Does the backslash at the end of the url matter?

Also, do you know how to add a token instead of basic auth username and password. I suggested using 'x-access-token' as the username and the password as the token like below:

username: x-access-token password: ${{secrets.REGISTRY_API_KEY}}

asciimike commented 3 years ago

Does the backslash at the end of the url matter?

No. The url field in registries is just a key to store the credentials by, the actual resolution of the dependencies is going to come from the cargo.toml, so I'd assume that something like:

# cargo.toml
[registries]
cargo-cloudsmith = { index = "https://dl.cloudsmith.io/basic/OWNER/REPOSITORY/cargo/index.git" }

[package]
name = "my-project"
version = "0.1.0"

[dependencies]
other-crate = { version = "1.0", registry = "cargo-cloudsmith" }

Should work, if you have a dependabot.yml that look like:

registries:
  cargo-cloudsmith:
    type: git
    url: https://dl.cloudsmith.io
    username: cargo-user
    password: ${{secrets.CARGO_GIT_PASSWORD}}
updates:
  - package-ecosystem: cargo
    directory: "/"
    schedule: 
      interval: daily
    registries:
     - cargo-cloudsmith

Also, do you know how to add a token instead of basic auth username and password.

I believe we only support HTTP basic auth, as that's what the GitHub git HTTP API uses, but I think it's a valid feature request to support Authentication: bearer <TOKEN> or similar (though I think we'd have a harder time supporting the use of arbitrary headers, e.g. x-api-key unless we broke the config out entirely. That said, it looks like cloudsmith supports HTTP basic, so it should work with any valid username/password, no?

ciaracarey commented 3 years ago

When I use a URL like that I get this error from Dependabot:

Dependabot failed to update your dependencies because there was an error resolving your Rust dependency files. ..... no index found for registry: 'cloudsmith-test-ciara-repo1'

Screenshot 2021-07-09 at 14 36 14
ciaracarey commented 3 years ago

Using url: https://dl.cloudsmith.io/

ciaracarey commented 3 years ago

Full dependancy.yml

registries:
   cloudsmith-cargo-basic:
     type: git
     url: https://dl.cloudsmith.io/
     username: ${{secrets.USERNAME}}
     password: ${{secrets.PASSWORD}}
 updates:
  - package-ecosystem: "cargo"
    directory: "/"
    registries:
      - cloudsmith-cargo-basic
    schedule:
      interval: "daily"
ciaracarey commented 3 years ago

Hi @asciimike, So I think I have made a little progress with the issue for the "no index found for registry" error  It looks like this error happens when the Cargo config for the registry is missing, normally this gets written into ~/.cargo/config, but since this is running in a GitHub actions like environment I get the error above.
 If I don't have access to the Config file I can set an environmental variable in a very specific format https://doc.rust-lang.org/cargo/reference/config.html#registriesnameindex

My environmental variable will be something like CARGO_REGISTRIES_CLOUDSMITH_TEST_CIARA_REPO1_INDEX=URL  Adding an environmental variable that Dependabot has access to isn't as obvious as I thought. This project successfully references an environmental variable secret in dependabot.yml as my password as ${{secrets.CARGO_GIT_PASSWORD}}

So, I added the CARGO_REGISTRIES_CLOUDSMITH_TEST_CIARA_REPO1_INDEX environmental variable secret to Settings->Secrets->Dependabot and I continued to get the same error- maybe it can't directly access the environmental variable- does it expect it to be preseeded by 'secrets' like the password above?.

Any advice on how to add an environmental variable that Dependabot can access would be great.

Thanks 

asciimike commented 3 years ago

Ok, I think my brain got a bit scrambled between cargo.toml and .cargo/config.toml; looks like we need to add support for .cargo/config.toml since registries lives there rather than in cargo.toml, and currently we only fetch cargo.toml. It sounds like this might be a pre-req for any sort of cargo private registry support (via git or otherwise).

We don't have the ability to plumb arbitrary env vars (the secrets are a pretty special case), and I feel like the former (parsing .cargo/config.toml is going to be much easier). At this point, beyond running dependabot-core yourselves, I think you'll be waiting on us implementing .cargo/config.toml support for registries. I've added an internal tracking issue for that.

ciaracarey commented 3 years ago

@asciimike thanks so much for getting back to us, Hopefully, it gets picked up as a nice easy win :)

ciaracarey commented 2 years ago

Hey guys, do you know if dependabot supports Cargo Private registries? or if there is a way to use arbitrary env var?

asciimike commented 2 years ago

Unfortunately we haven't been able to prioritize this work, and probably won't be able to for at least another quarter.

There isn't a way to do arbitrary env vars at present.

jeffwidman commented 1 year ago

I closed #1767 as a dupe of this, since this has a little more implementation details.

There is a fork of Dependabot mentioned in https://github.com/dependabot/dependabot-core/issues/1767#issuecomment-607224973 where @johnbatty added the necessary bits to talk to private registries.

Also two crates that @lilymara-onesignal put together for initial testing/debugging purposes: https://github.com/dependabot/dependabot-core/issues/1767#issue-589416128

We're definitely interested in PR's if someone wants to take a crack at this (hint @johnbatty 😄 ).

However, I should mention that within GitHub we use an internal proxy for handling credentials so that they're never exposed to untrusted user code while evaluating package managers... ie, landing on dependabot-core is step 1, but before it can fully land on Dependabot the service we'll still need to do step 2 of teaching the proxy how to talk to the registry.

That said, since Dependabot can also be wired up to run as a GitHub action, once the code lands in dependabot-core, then running it via an action is a viable workaround for talking to private registries.

CodingAnarchy commented 8 months ago

I see from the documentation here that a token is an option for the arguments. Does this mean that we can set this up now, or is there still something barring the way? Trying to clarify before doing a bunch of configuration only to find out this ticket is still open for a reason.

marosset commented 8 months ago

Alternate registry authentication has stabilized in the Rust toolchain (x-ref https://github.com/rust-lang/cargo/issues/10474 for the tracking issue). I am working in some projects that current require pulling some packages from private cargo feeds and dependabot is failing to resolve Rust dependencies in these repositories and unfortunately git references won't work for me either (I need push some packages to the same private registry).

Are there any plans to add support here now that everything is private registry auth is stable?

danielhaap83 commented 8 months ago

Ok, I think my brain got a bit scrambled between cargo.toml and .cargo/config.toml; looks like we need to add support for .cargo/config.toml since registries lives there rather than in cargo.toml, and currently we only fetch cargo.toml. It sounds like this might be a pre-req for any sort of cargo private registry support (via git or otherwise).

We don't have the ability to plumb arbitrary env vars (the secrets are a pretty special case), and I feel like the former (parsing .cargo/config.toml is going to be much easier). At this point, beyond running dependabot-core yourselves, I think you'll be waiting on us implementing .cargo/config.toml support for registries. I've added an internal tracking issue for that.

It feels like I am running exactly into this configuration mismatch. I am using a private registry and the registries index is configured in the .cargo/config.toml file. In the Cargo.toml I just referenced the registry via

...
[dependencies]
name = { version = "...", registry = "..." }
...

Cargo is able to resolve this because it knows the registries index by parsing the .cargo/config.toml file. But dependabot seems to ignore this. Error from dependabot is:

updater | 2024/01/11 08:33:55 INFO <job_3566> Handled error whilst updating derive-new: dependency_file_not_resolvable {:message=>"error: failed to parse manifest at `dependabot_tmp_dir/Cargo.toml`\n\nCaused by:\n  no index found for registry: `...`"}

Is there any plan to fix this?

BR, Daniel

CodingAnarchy commented 8 months ago

@danielhaap83 That change is included in #8719. I'm waiting on reviews right now for the full change to support private registries there.

ciaracarey commented 7 months ago

Hey @CodingAnarchy do you want me to test out anything?

mctofu commented 6 months ago

Support for fetching .cargo/config.toml was recently added in https://github.com/dependabot/dependabot-core/pull/9109. That may unblock usage of git indexes using Dependabot's git registry authentication?

Alternate registry authentication may be a little tricky. Dependabot on GitHub (and via the CLI) doesn't provide authentication directly to package managers and instead applies authentication to network requests with a http proxy. If the cargo command fails when it can't find credentials that would cause updates to fail even if we added support to the proxy. In that case there'd be some extra work to provide some fake credentials or find a way to disable the credential requirement?

abdulapopoola commented 6 months ago

@ciaracarey ; would it be possible to get some testing done please? @mctofu and @pavera shipped some changes towards this goal this week.

mctofu commented 6 months ago

I think I've added support for sparse cargo registry authentication to our proxy but I don't have access to a registry to test against. It's currently available to try via the Dependabot CLI. I expect there may be further changes needed in the updater to get this fully working but this should help determine what's needed in https://github.com/dependabot/dependabot-core/pull/8719.

To try it out, first install the CLI.

Create a job.yml file as follows. Replace the source repo with your own as well as the url and token for the cargo credentials. The proxy will apply authentication to requests matching the host of the url and under the path of the url. The value of the token will be directly set as the Authorization header so if the cargo credentials are like Bearer <token> use that full value as the token here.

---
job:
  package-manager: cargo
  allowed-updates:
  - dependency-type: direct
    update-type: all
  source:
    provider: github
    repo: dependabot-fixtures/cargo-test
    directory: "/."
    branch: main
    api-endpoint: https://api.github.com/
    hostname: github.com
credentials:
  - type: cargo_registry
    url: https://example-cargo-registry.dev/optional-path
    token: Bearer not-a-real-token

If the source repo is private, set an env variable named LOCAL_GITHUB_ACCESS_TOKEN to a github PAT which can read the repo.

Run dependabot update -f job.yml and verify you can get updates to the private dependency.

To use the CLI to test changes to core, check out the debugging guide.

edit: Note this is only for sparse registries. If using a git registry then use the existing guidance for configure authentication for git: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#git

ciaracarey commented 6 months ago

Absolutely, I'll bring this on Monday to the support team

On Fri 1 Mar 2024, 19:08 AbdulFattaah Popoola, @.***> wrote:

@ciaracarey https://github.com/ciaracarey ; would it be possible to get some testing done please? @mctofu https://github.com/mctofu and @pavera https://github.com/pavera shipped some changes towards this goal this week.

— Reply to this email directly, view it on GitHub https://github.com/dependabot/dependabot-core/issues/3478#issuecomment-1973766099, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUB2CFN2HDUPNCL7B2PJHVDYWDG2NAVCNFSM42ZYJKQ2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJXGM3TMNRQHE4Q . You are receiving this because you were mentioned.Message ID: @.***>

RobJellinghaus commented 4 months ago

@mctofu were you able to get any validation of this? I am interested in helping get private registries working end-to-end and I have a test case Azure DevOps repo I'm working with internally. I will try your above instructions out against that repository and report back.

abdulapopoola commented 4 months ago

That's great to know @RobJellinghaus ! Let us know how it goes.

cc: @carlincherry as fyi and @jonjanego too

RobJellinghaus commented 4 months ago

I believe I have set up the correct reproduction of this locally, using my own fork of #8719 merged with current dependabot-core main. Unfortunately it is not working:

  proxy | 2024/04/29 21:05:58 [008] GET https://pkgs.dev.azure.com:443/mscodehub/Rust/_packaging/Rust_CrateReview/Cargo/index/config.json
  proxy | 2024/04/29 21:05:58 [008] 401 https://pkgs.dev.azure.com:443/mscodehub/Rust/_packaging/Rust_CrateReview/Cargo/index/config.json
  proxy | 2024/04/29 21:05:58 [008] Remote response: {"$id":"1","innerException":null,"message":"TF400813: The user 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' is not authorized to access this resource.","typeName":"Microsoft.TeamFoundation.Framework.Server.UnauthorizedRequestException, Microsoft.TeamFoundation.Framework.Server","typeKey":"UnauthorizedRequestException","errorCode":0,"eventId":3000}

This may be a token encoding issue, or something else.

My dependabot-cli YAML configuration is:

job:
  package-manager: cargo
  allowed-updates:
  - dependency-type: direct
    update-type: all
  #experiments:
  #  record-ecosystem-versions: true
  source:
    provider: azure
    repo: mscodehub/Rust/_git/rust.crate-knowledge
    directory: "/."
    branch: user/rjelling/1.75
  credentials:
  - type: cargo_registry
    url: https://pkgs.dev.azure.com/mscodehub/Rust/_packaging/Rust_PublicPackages/Cargo/index
    token: Bearer [pat]
  - type: cargo_registry
    url: https://pkgs.dev.azure.com/mscodehub/Rust/_packaging/Rust_CrateReview/Cargo/index
    token: Bearer [pat]
  - type: git_source
    host: dev.azure.com/mscodehub
    username: x-access-token
    password: [pat]

@mctofu can you please provide a link to the PR where this proxy support was added? I would love to debug it but I haven't been able to locate your change. Thanks very much.

CodingAnarchy commented 4 months ago

@mctofu I finally managed to test #8719 with Dependabot CLI and this appears to have worked to resolve the expected updates. The output (trimmed of the network request logs, etc.) is below:

updater | +--------------------------------------------------+
updater | |       Changes to Dependabot Pull Requests        |
updater | +---------+----------------------------------------+
updater | | created | deadpool ( from 0.10.0 to 0.11.2 )     |
updater | | created | mockall ( from 0.11.4 to 0.12.1 )      |
updater | | created | serde ( from 1.0.198 to 1.0.199 )      |
updater | | created | strum ( from 0.25.0 to 0.26.2 )        |
updater | | created | strum_macros ( from 0.25.3 to 0.26.2 ) |
updater | | created | base64 ( from 0.21.7 to 0.22.0 )       |
updater | | created | tower-http ( from 0.4.4 to 0.5.2 )     |
updater | +---------+----------------------------------------+

The job.yml I used was as follows:

---
job:
  package-manager: cargo
  allowed-updates:
  - dependency-type: direct
    update-type: all
  source:
    provider: github
    repo: Shopify/shopify-cdp
    directory: "/grpc"
    branch: main
    api-endpoint: https://api.github.com/
    hostname: github.com
credentials:
  - type: cargo_registry
    registry: shopify-rust
    url: "https://cargo.cloudsmith.io/shopify/rust/"
    token: "Token [pat]"

Without the registry key in the job.yaml, this failed to pick up the registry when trying to inject the tokens for cargo: ERROR undefined method 'upcase' for nil:NilClass ENV["CARGO_REGISTRIES_#{cred['registry'].upcase.tr('-', '_')}_TOKEN"] ||= "Token #{cred['token']}". That key does not seem to be necessary if that auth injection was removed, though.

I tried to remove the auth work as suggested in https://github.com/dependabot/dependabot-core/pull/8719#discussion_r1509576476, but that failed on the requests where those tokens are injected (before the RegistryClient call and for the cargo update calls in the ecosystem). I may not have removed those injections properly, so that aspect is still open.

I plan to update #8719 to resolve the conflicts it now has with dependabot-core main branch this week and test again, just to be sure.

jrudolph commented 4 months ago

Can someone clarify what the state is? With #8719 merged, is private registry support coming to the Github dependabot service (dependabot.yml) or only for manual CLI usage?

niels-s commented 4 months ago

I reached out to GH support last week because support for GH Dependabot still needs to be implemented. There is a public roadmap issue: https://github.com/github/roadmap/issues/945, which mentions that it will add support in Q2. So, in the worst case, we have to wait two months 🤞

carlincherry commented 3 months ago

This feature launched today! https://github.blog/changelog/2024-06-03-dependabot-now-supports-private-cargo-registries/