cargo must not package arbitrary files

[rillian]

Today I learned 'cargo package' includes everything in the working directory not explicitly excluded through .gitignore or the package.excludes directive in Cargo.toml. This wasn't what I expected at all, and differs from what, for example, GNU autotools implements as best practice. I expected cargo to only package files which were part of of the build description and its dependent source.

I only discovered this because I copied a few test videos into my working tree and the package was suddenly too large to upload.

In #1597 @bluss suggests a warning for untracked files. That would be an improvement, at perhaps educate people about the default, but I think that doesn't go far enough. Uploading random files from the working directory is a terrible developer experience, and
makes is very easy to leak confidential or unlicensed data.

[achanda]

Doesn't this solve your problem? http://doc.crates.io/manifest.html#the-exclude-and-include-fields-optional

[rillian]

No. It lets me work around problematic behaviour, now that I know about it. The problem is the default.

[gkoz]

I too would've been surprised by cargo picking up any junk in the working directory. I'd appreciate it aborting with an error unless a command line override is given.

In the absence of .git I'd expect the set of "tracked" files to be ones "reachable" from the manifest (explicitly included or participating in the build).

[lukaslueg]

I've done a check on all 3.248 crates and 17.085 tarballs on crates.io as of 15/10/24, checking
for the the following problems:

• Members in the tarball that are not files (e.g. symlinks, devices, etc.)
• Huge files (>50mb) and gzip-bombs
• Pre-fixed UID/GID/username/groupname
• Hidden files and paths, usually a sign of unintended inclusion (e.g. 'src/.vimrc')
• Absolute paths (e.g. '/etc/passwd')
• Path escapes (e.g. 'foobar-0.1.0/src/../../../../../../etc/passwd')
• Local build and editor files (e.g. ".git", ".dockerignore", "*.swp" and
  around 20 more)
• Visible .cargo/config, possibly including the API-token
• Around 50 checks for possibly sensitive files like private ssh keys, shell
  history files, keychains, netrc, s3cmd configuration files etc.

The good news is that as far as I can see there are no serious problems as of
now. There are however some issues that motivate to dig deeper here:

1. Roughly one gazillion files have their owner explicitly set.
   This is not a serious problem, it might leak usernames though, but is
   embarrasingly easy to fix in cargo.
2. Roughly one gazillion files are set executable by either the owner, the
   group or other. This again is not a serious problem and easy to fix in
   cargo. It's always a sign of good housekeeping if your LICENSE is not set executable...
3. 17 crates include a .cargo/config, none of them include the
   API-token. This should never happen.
4. Around 12.000 local build- or editor-related files and directories are
   present, mostly ".git", ".gitted" and ".travis.yml". Sometime entire git- or
   mercurial repositories are included in the final crate. There are also lots and
   lots of editor-related temporary files which may or may not include sensitive
   data.
5. Three huge files where found, ranging from 67mb to 79mb. All three were
   included by accident, are not present in the repository and compress so well
   that they don't bust the maximum crate size.
6. 17 crates point to invalid tar files, usually tarballs without
   members. All of them have version numbers like '0.0.1'. No path-escapes,
   absolute paths, fishy member types or gzip-bombs were found.
7. 47 possibly sensitive files were found, all of them either false positives
   or test-related files (mostly crypto-related crates including private
   certificate files or similar).
8. Some small amount of various hidden files and paths not related to editors were found (e.g. ".cargo-old/")
9. Some small amount of trash-files (.DS_Store) were found

The full table with all crate-lint-warnings can be found below.

My two cents of all this:

• Cargo itself Has. To. Be. more careful about what it includes in the crate.
• My proposal is to include some easy to understand, reliable rules based on
  known filenames and patterns that cause cargo to reject the build if those
  files are about to get pulled in and not part of the Include-rules. Adding the
  files to the Include-rules turn the errors into warnings.
• We can check the interference caused by the proposed linter on the already
  existing crates.
• The above could be introduced gently by raising warnings in the next stable
  version of cargo and turning the warnings into errors the version after.
• Crates.io also has to do some checking on the crates it was given; after all
  we can't assume that all tarballs are uploaded by Cargo. We should at least
  check for the most serious problems, some of which are mentioned above.

If someone is interested in the linter's code I've written over the weekend, I
can clean it up and post it somewere if poked...

All the gory details:

Type	Info	Count
CargoConfig	None	17
CargoConfig Total		17
HiddenFile	._hmtool_Info.plist	1
	._macsupport.h	1
	._macsupport.m	1
	._samples_c_Info.plist	1
	._samples_cpp_Info.plist	1
	.astylerc	1
	.build.sh	1
	.buildpacks	1
	.dir-locals.el	1
	.dntrc	1
	.docs.sh	1
	.eslintignore	1
	.fac	1
	.hg_archival.txt	1
	.htaccess	1
	.jot	1
	.jsbeautifyrc	1
	.jscs.json	1
	.leading.dot.txt	1
	.load.png	1
	.npmrc	1
	.sconsign.dblite	1
	.store.png	1
	.suo	1
	.swap.png	1
	.travis-doc-conf.sh	2
	.travis-doc-key.enc	2
	.travis.install.deps.sh	1
	.travis.yml.disable	1
	.travis.yml~	1
	.zuul.yml	1
HiddenFile Total		33
HiddenPath	.bin	15
	.cargo	18
	.cargo-old	1
	.cargone	1
	.crusader	1
	.dave	1
	.images	3
	.sconf_temp	2
	.tx	1
	.vs	1
HiddenPath Total		44
HugeFile	67108864	2
	79140779	1
HugeFile Total		3
LocalBuildFiles	.add.rs.swp	1
	.address.rs.swp	1
	.any_against_any.rs.swp	1
	.appveyor.yml	4
	.arcconfig	1
	.asm.rs.swp	1
	.ast_util.rs.swp	1
	.ast.rs.swp	1
	.atom-build.json	4
	.attr.rs.swo	1
	.attr.rs.swp	1
	.audio.rs.swp	1
	.auto.rs.swp	1
	.ball_against_ball.rs.swp	1
	.base.rs.swp	1
	.base58.rs.swp	1
	.bindgen.rs.swp	1
	.blocks.rs.swp	1
	.build.rs.swp	4
	.bumpversion.cfg	2
	.bytes.rs.swp	1
	.Cargo.lock.swp	1
	.Cargo.toml.swn	1
	.Cargo.toml.swo	2
	.Cargo.toml.swp	89
	.Cargo.toml.un~	1
	.Checklist.md.swp	1
	.ci	5
	.clang-format	2
	.classpath	1
	.clog.toml	10
	.codemap.rs.swp	1
	.comments.rs.swp	1
	.composite_shape_against_any.rs.swp	1
	.config.swp	1
	.connection.rs.swp	1
	.controller.rs.swp	1
	.coveralls.yml	1
	.cp.rs.swp	1
	.de.rs.swp	4
	.decoder.c.swp	1
	.deps	338
	.diagnostic.rs.swp	1
	.dirstamp	40
	.Dockerfile.swp	1
	.dockerignore	1
	.drone.yml	2
	.editorconfig	23
	.emacs.bmk	3
	.eq.rs.swp	1
	.error.rs.swp	2
	.eslintrc	4
	.example.rs.swp	2
	.expand.rs.swo	1
	.expand.rs.swp	1
	.ffi.rs.swp	2
	.fingerprint	54
	.fold.rs.swo	1
	.fold.rs.swp	1
	.foo.rs.swo	1
	.foo.rs.swp	1
	.foo2.rs.swp	1
	.format.rs.swp	1
	.frigg.yaml	1
	.funcs.rs.swp	1
	.gdb_history	1
	.gdbinit	1
	.gitattribute	1
	.gitattributes	47
	.gitauthors	1
	.gitconfig	3
	.gitignore	2993
	.gitkeep	103
	.gitlab-ci.yml	3
	.gitmodules	101
	.gitreview	4
	.gitted	5737
	.gjk.rs.swp	1
	.H5Tpublic.h.swp	1
	.haptic.rs.swp	1
	.hg	419
	.hgignore	4
	.hgtags	2
	.idea	15
	.impls_core.rs.swp	1
	.impls_libstd.rs.swp	1
	.impls_vec_map.rs.swp	1
	.impls.rs.swo	1
	.impls.rs.swp	2
	.iter.rs.swp	1
	.jshintrc	9
	.kateproject	1
	.lib.rs.swo	5
	.lib.rs.swp	33
	.lib.rs.un~	1
	.libs	214
	.LICENSE.swp	1
	.linux.rs.swo	1
	.macro_parser.rs.swp	1
	.macro.rs.swp	2
	.main.rs.swp	1
	.Makefile.swp	1
	.method.rs.swp	1
	.mod.rs.swo	4
	.mod.rs.swp	7
	.npmignore	56
	.ord.rs.swp	1
	.parser.rs.swo	1
	.parser.rs.swp	1
	.partial_eq.rs.swp	1
	.plane_support_map.rs.swp	1
	.pp.rs.swp	1
	.pprust.rs.swn	1
	.pprust.rs.swo	1
	.pprust.rs.swp	1
	.project	23
	.projectile	1
	.ray_aabb.rs.swp	1
	.ray_ball.rs.swp	1
	.ray_plane.rs.swp	1
	.ray_support_map.rs.swp	1
	.README.md.swp	6
	.rspec	1
	.rustc.rs.swp	1
	.ser.rs.swo	1
	.ser.rs.swp	1
	.settings	3
	.shape_against_shape.rs.swo	2
	.sigar_shellrc	1
	.snakemake	51
	.source_util.rs.swp	1
	.str.rs.swp	1
	.support_map_support_map.rs.swp	1
	.swo	1
	.swp	3
	.syncthing..travis.yml.tmp	1
	.tags	1
	.tags1	1
	.testass.rs.kate-swp	1
	.travis	44
	.travis_after.sh	1
	.travis-bench	1
	.travis-update-gh-pages.sh	3
	.travis.sh	2
	.travis.yaml	1
	.travis.yml	1510
	.travis.yml.swl	1
	.travis.yml.swp	1
	.types.rs.swp	1
	.valgrind.supp	1
	.value.rs.swp	2
	.vimrc	2
	.visit.rs.swo	1
	.visit.rs.swp	1
	.wordcut.rs.swp	1
LocalBuildFiles Total		12112
SensitiveFile	Configuration file for auto-login process	1
	Contains word: password	7
	Contains words: private key	2
	Network traffic capture file	2
	Potential cryptographic key bundle	14
	Potential cryptographic private key	21
SensitiveFile Total		47
TarError	None	17
TarError Total		17
Trash	.DS_Store	20
	.HEADER	3
	.keep	1
	.lock	6
	.mailmap	5
	.name	2
	.timestamp	101
	.version	2
Trash Total		140
Grand Total		12413

[lukaslueg]

Two crates currently have their owner's API-token exposed by a strace-dump having being pulled into the tarball. The owner has recreated his token by now.

Any recommendations on how to proceed on this?

[alexcrichton]

Another comment by @felixc on #2245:

---

I'm not sure whether to file this as a bug, a request for clearer documentation, or a suggestion for concrete changes in behaviour. Regardless, I wanted to bring up how cargo package and cargo publish implicitly grab all local files in the directory and include them in the crate. (Yes, because I was just caught out by this, though fortunately I don't believe I've published anything sensitive — but I'm about to go and double-check all my old versions).

What I'd like to argue is that implicitly grabbing all files in the local directory and including them in the published crate is a very dangerous feature. At least my mental model of "the contents of my crate" consists of just source files, docs, tests, and ancillary files like README and LICENSE; not of everything that happens to be in the same directory.

Like the Python folks say, "explicit is better than implicit". The safe default behaviour would be to require an explicit listing of what files to include (yes, I know the option exists, but it's not the default). If you'd like to encourage people to remember to keep it up to date, you could print a warning when you see e.g. a LICENSE or CONTRIBUTING file that isn't explicitly listed, or even any new file found for the first time. Having everything grabbed would work better as an opt-in for those who know about the feature and choose to accept the risk, but defaults should be chosen so as to fail in a safe manner.

If, on the other hand, this behaviour is really too desirable to give up, it should be very very explicitly called out, and loudly signalled (i.e. printed to stdout) when it does happen.

Currently the only mention of it that I can find is in the crates.io doc here, and even that isn't particularly explicit or clear. Firstly, I never though of my TODO list for the crate, my personal notes on my release process, etc. as "part of the crate", and thus publishable. Secondly, though it says "Cargo will automatically ignore files ignored by your version control system when packaging", I assumed that meant "files not tracked by your version control system", rather than specifically "files listed in .gitignore" (the only supported ignore mechanism, as far as I can tell).

But, most of all, it's a big scary footgun feature that's just mentioned in passing in one skimmable section (I admit I went straight to cargo upload the first time I read this, and skipped package altogether).

So, in sum, please reconsider the default behaviour of this feature, and please aim for safety over convenience, even if convenience is offered as an option for those who choose it.

[lambda]

A suggestion I would make would be to change the semantics of include and exclude in the manifest, so that files must be explicitly included via include, and that exclude would apply on top of that to then exclude files that would otherwise be included (so you could do include = "src/**/*" but also exclude = *~ to exclude your editor files, for instance). This would be a breaking change, as right now include and exclude are mutually exclusive, but I don't believe that Cargo has hit 1.0 yet.

Another suggestion would be to add some kind of further version control integration so that it can take advantage of something like git archive/hg archive/svn export to produce a pristine tarball or directory that would be used as the basis for upload. This would also generally have the advantage of uploading the contents of the head commit, rather than just whatever is in the working directory which may be dirty.

Of course, this wouldn't then be able to include any files generated as part of a build process that aren't in source control, and would add more coupling to particular version control systems. It could be made to work more generally and without the problem of not being able to use built steps by just having something like export-command = "git archive > $@" or export-command = "hg export $@; cd $@; big-complicated-build-step, which would be expected to produce a directory or tar archive.

Note that both of these suggestions could be useful independently. Using an export command can help when using a VCS to ensure that you only include an actual committed version rather than something dirty; but it doesn't help you exclude files that are in version control but you don't want to release (there's no reason your .gitignore, .gitattributes, etc should be included in the crate, but they need to be included in version control).

Always requiring a whitelist, and having the blacklist apply on top of that, would make it much safer to use, but it wouldn't fix the problem of people accidentally releasing some uncommitted state and then not being able to reproduce that state later from version control. Yes, you'd have the opposite problem of people sometimes uploading crates without everything included, but that's generally failing safer than people uploading too much, as if not everything is included you'll generally notice and can then fix it, while you can never un-publish secrets.

[lukaslueg]

From time to time I pull new and updated crates from the index and scan them for possibly sensitive files and contents. I posted the stats from 2015/10 above, the index now has ~3.800 crates and ~20.600 tarballs. So far three crates had their private API token exposed through an strace-dump that was pulled into the tarball. The most "severe" problem with crate-hygiene are local build-, trash- and vcs-files.

I would suggest to leave the include- / exclude-semantics as they are for now but add some hygiene-checking that results in warnings by cargo when building the package. Future version may turn some or all warnings into errors, forcing the user to explicitly include the offending files. Some of the things that should result in a warning:

• Known sensitive file names (e.g. ".*rsa\Z" or ".?(bash|zsh_|z)?history\Z")
• Known patterns in files (e.g. "AKIA[0-9A-Z]{16}" or "-----BEGIN (RSA )?PRIVATE KEY-----")
• Known vcs/build/editor files
• Huge files
• Cargo's own files

To produce the stats in the lengthy comment above, I hacked a linter in python (https://github.com/lukaslueg/lint_crates/). It checks for dozens of unhygienic files, possible tarball/gzip-bombs and such. One might simply port that to Rust.

[lukaslueg]

In the meantime, @alexcrichton leaked his API-key in toml-0.1.26 due to a strace-dump getting pulled into the crate; he revoked his API-key.
@sp3d also lost his API-key in tilde-expand-0.1.0 due to the full .cargo/config getting sucked in; since there is no contact info for @sp3d, I've simply yanked his crate. Maybe someone with 0222 on crates.io could remove the crate altogether?

[vks]

I think the current behavior is unnecessarily error-prone. The packaged files should be opt-in rather than opt-out.

[sp3d]

Oh. API key reset. Thanks for the heads-up. I had no idea that footgun even existed and was blindly following instructions on crates.io w/r/t publishing. I had assumed Cargo would only package things tracked by git, and didn't realize it would take all contents of the directory, including .cargo (which was there rather than in ~ because I had set CARGO_HOME to the top-level directory of the repository).

Now that I read the documentation (http://doc.crates.io/manifest.html), it seems really weird that Cargo would use the exclude directives given to git et cetera (e.g. .gitignore) but not the include information (which files are tracked by the repository).

I would expect documentation, either in the "The exclude and include Fields (optional)" section or in the "The Project Layout" section, to state that Cargo considers all files in the directory to be part of the package and will publish them, unless exceptions are configured.

It also seems strange that the default set of files (all of them) may be subtracted from using exclude, but the include field replaces (rather than adding to) this set, and cannot be subtracted from using exclude ("setting include will override an exclude"). This means that Cargo's current behavior cannot be seen as include simply having a default value of ["*"], because that would prevent exclude from working at all.