Skip to content
This repository has been archived by the owner on Oct 27, 2023. It is now read-only.

Evaluate choice in package IDs #412

Closed
AArnott opened this issue Feb 5, 2021 · 22 comments · Fixed by #478
Closed

Evaluate choice in package IDs #412

AArnott opened this issue Feb 5, 2021 · 22 comments · Fixed by #478
Assignees

Comments

@AArnott
Copy link

AArnott commented Feb 5, 2021

All Microsoft internal tools packages should have IDs that begin with Microsoft. so that their absence from nuget.org does not present a dependency confusion vulnerability to repos that consume it from one feed and also consume nuget.org via nuget.config.

@AArnott
Copy link
Author

AArnott commented Feb 5, 2021

@jonfortescue, I hear you're the one to look at this.

@jonfortescue
Copy link
Contributor

This is not actually related to my epic. Kicking back to triage.

@jakubstilec
Copy link
Contributor

[Async Triage]: @AArnott please could you add description to have more context?

@AArnott
Copy link
Author

AArnott commented May 13, 2021

The description was blank before to avoid disclosing a vulnerability before it was announced. I filled it in now.

@riarenas
Copy link
Member

FYI @mmitche for guidance

@mmitche
Copy link
Member

mmitche commented May 13, 2021

Yes, we should make this the prefix.

@riarenas
Copy link
Member

@markwilkie @Chrisboh @tkapin How do we want to handle these kinds of changes in Xliff-tasks? It's not a giant change so I'm leaning towards throwing this in FR, it requires changing the package name and then making sure arcade depends on the new name.

@markwilkie
Copy link
Member

Seems to me this is something we should do from a compliance perspective - so ya, FR. @ilyas1974 ?

@ilyas1974
Copy link

I agree. Seems like something that we can take care of in FR.

@ilyas1974
Copy link

@jonfortescue what is the status of this?

@jonfortescue
Copy link
Contributor

@ilyas1974 this was meant to be handed off to the next person on FR but I was absent the day of hand-off; sorry about that. A PR is in (#478) but I'm worried about the consequences changing the name will have.

@AArnott
Copy link
Author

AArnott commented Jul 20, 2021

Any specific concerns? Generally speaking, a new ID doesn't break any existing users. It does make it harder for them to notice that newer versions are available (on another ID) the first time.

@lpatalas
Copy link
Contributor

I'm currently on FR and I will take over this issue from Jon.

@lpatalas lpatalas assigned lpatalas and unassigned jonfortescue Jul 21, 2021
@riarenas
Copy link
Member

I don't think the name change is too concerning. For the most part, repos get xliff from arcade, so we will need to make sure arcade still gets updated with the new package version.

I think all we wound need is:

@lpatalas
Copy link
Contributor

Thanks for the tips @riarenas. I double checked @jonfortescue's changes and they seem ok. Build and tests finish successfully. I'm not sure if I can do any more due diligence there so I think we just need to merge it and see if the package is published correctly.

Is there someone specific I should put as a reviewer? Currently I've added @riarenas and @MattGal.

@lpatalas
Copy link
Contributor

Based on @mmitche suggestion I renamed the package to Microsoft.DotNet.XliffTasks. I've already updated the PR #478. After it's merged we need to make changes in Arcade that @riarenas described above.

@premun
Copy link
Member

premun commented Jul 26, 2021

I see this package is used in several repos, I will open a PR in each so that people know about the name change:

  • roslyn-tools
  • arcade
  • sdk
  • upgrade-assistant
  • machinelearning

@jonfortescue
Copy link
Contributor

@premun Just so you know, the package is used in dozens of repos that pull it in through arcade. All of the repos in this issue, for example, use xliff-tasks.

@riarenas
Copy link
Member

If they bring it through arcade they shouldn't need any changes beyond having arcade use the new name.

premun added a commit to dotnet/arcade that referenced this issue Jul 27, 2021
The package name was changed to start with a reserved name (dotnet/xliff-tasks#412)
@premun premun assigned greenEkatherine and unassigned lpatalas and premun Aug 2, 2021
@greenEkatherine
Copy link

PR merged, is there anything else to do?

@AArnott
Copy link
Author

AArnott commented Aug 7, 2021

Looks good to me. Thank you.

@ilyas1974
Copy link

Closing issue - if anything else is needed, please let us know and we will address it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants