Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Failing spec for #3671#3696

Closed
TimMoore wants to merge 1 commit into1-9-stablefrom
issue-3671
Closed

Failing spec for #3671#3696
TimMoore wants to merge 1 commit into1-9-stablefrom
issue-3671

Conversation

@TimMoore
Copy link
Copy Markdown
Contributor

No description provided.

@segiddins
Copy link
Copy Markdown
Contributor

Did we ever make any progress on fixing this?

@indirect
Copy link
Copy Markdown

Not yet...

On Mon, Aug 17, 2015 at 9:59 PM, Samuel E. Giddins
notifications@github.com wrote:

Did we ever make any progress on fixing this?

Reply to this email directly or view it on GitHub:
#3696 (comment)

@TimMoore
Copy link
Copy Markdown
Contributor Author

I made a failed attempt a while back... it's a bug whack-a-mole. I managed to fix this spec but broke several others 😭

end
end

it "installs the dependency from the top-level source without warning" do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimMoore shouldn't it install from the pinned source here rather than the top-level source?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this case, I think, because the thing that depends on it comes from the top-level source.

but then I don't know what should happen when you have multiple gems from multiple different sources that depend on a third gem

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, ended up getting it working :D

homu added a commit that referenced this pull request Jun 24, 2016
[2.0] Remove RubyGems Aggregate & support transitive source pinning

Closes #3671.
Closes #3696.
homu added a commit that referenced this pull request Jun 28, 2016
[2.0] Remove RubyGems Aggregate & support transitive source pinning

Closes #3671.
Closes #3696.
Closes #4059.
@segiddins
Copy link
Copy Markdown
Contributor

This PR will be auto-closed once 2-0-dev is merged to master

bundlerbot added a commit that referenced this pull request Jun 27, 2017
[2.0] Remove RubyGems Aggregate & support transitive source pinning

### What was the end-user problem that led to this PR?

The problem was that the resolver could resolve specs from _any_ of the sources specified in the Gemfile, even if that source had nothing to do with the spec in question. This was such a large security vulnerability that, when discovered, it warranted a CVE and its own minor release of Bundler.

Closes #3671.
Closes #3696.
Closes #4059.

### Was was your diagnosis of the problem?

My diagnosis was that we needed to get rid of the notion of a `rubygems aggregate` and enforce that specs could only come either from the source they were declared to come from (the top-level source if declared at the top-level of the Gemfile, else a scoped source), or a source that it transitively "inherited" from the gems that required it.

### What is your fix for the problem, implemented in this PR?

My fix is to disable multiple top-level sources in the Gemfile, remove the RubyGems aggregate, and filter the sources gems could come from as described above.

### Why did you choose this fix out of the possible options?

I chose this fix because it allows doing the filtering in a reasonably performant manner, and refactors the way we handle sources to abstract some of the grossness in such a way that the machinations to make sure that all of the necessary gem info is downloaded is encapsulated into a single method, driven from the definition, rather than being specific to rubygems sources.

See #4714 and #4930 for the prior implementation.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants