Pitch: Disable checks for unsafe flags in SwiftPM

Currently, only a small set of compiler and linker flags in tagged package versions are considered “safe” for consuming packages. These include compiler defines, relative header search paths for C modules, and to enable various Swift language features. Very recently we added settings for warnings as safe. All other options must be declared unsafe.

Unfortunately when you try to add a dependency on products with targets that have unsafe flags, you will get a build error. The original intention of the unsafe flags feature was to ensure they didn’t adversely affect projects consuming those packages. However, what makes a flag unsafe was never fully defined. And, In fact, there are many flags that packages should be able use, and in some cases must be used, that are actually safe. This has caused a lot of frustration in our community and prevented many projects from doing proper releases.

As an example, the swift-java package needs to add include path flags to pick up the Java Native Interface header files. The location of these headers are extracted from an environment variable and are absolute paths which SwiftPM considers unsafe. So you end up with target settings like this:

swiftSettings: [
    .swiftLanguageMode(.v5),
    .unsafeFlags([
        "-I\(javaIncludePath)",
        "-I\(javaPlatformIncludePath)"
    ])
]

The Swift Java community accepts that the environment variable will always be defined and they do not consider this to be unsafe. The restriction is an obstacle to swift-java adoption. Enterprise consumers really need to depend on fixed release tags to ensure they can be confident packages they depend on are tested and stable. This is an example of where the restriction actually leads to unsafe practices.

For Swift 6.2, I would like to remove this check in SwiftPM. It's a simple change being late in the release cycle, but one that will be a great help to our ecosystem. For the following release, I would like to put together a Swift Evolution proposal to adopt a new name for these flags that doesn't lead to misconceptions about their safety-ness.

I look forward to your feedback on this proposal and if we have agreement, thoughts on what the name should be for these settings, since "unsafe" has a special place in the Swift world and shouldn't be used without clarity.

27 Likes

send it, this check has been deeply annoying and a major limitation of SwiftPM for too long

10 Likes

This sounds great to me - Foundation and our packages have been in a similar position where even adopting simple, core language features was blocked by this restriction that required unfortunate workarounds (see [Frontend] Add AvailabilityMacro feature by Azoy · Pull Request #65218 · swiftlang/swift · GitHub where we had to make things experimental features to work around the restrictions on additional unsafe flags). Agreed that in a future release an evolution proposal about the API itself would be great but I'm all for disabling the restriction on its usage in SwiftPM for this release to unblock both adoption of new language features via compiler flags as well as allowing for these capabilities which in many cases are safe.

1 Like

I was going to propose a similar change, and I generally support that direction. But we have to be careful because allowing arbitrary flags from dependencies increases the attack surface.
For example, a malicious package may load an arbitrary evil clang plugin, which would lead to arbitrary code execution.
I was thinking about a system where dependency declares a set of unsafe flags it wishes to use, and the dependent package declares an allowlist per package. And SwiftPM then verifies that the dependency doesn't attempt to use something funny.

2 Likes

Another major benefit of this is that it simplifies the API design surface for custom flags in SwiftPM (by removing it almost entirely). A big annoyance for SwiftPM's and the compiler's evolution is that new flags that one might want to support in both (like the changes to default isolation) need to contrive an additional "safe" way to spell it in SwiftPM that's close-to-but-subtly-different from the compiler flag.

In some cases, SwiftPM may want/need to be aware of certain flags (to do validation, or to change build planning in the presence of those flags), so it makes sense to have specific APIs for them so that it doesn't have to scrape a list of textual flags. Bazel is similar—we let users write library_evolution = True instead of copts = ["-enable-library-evolution"] because we need to account for an additional output file that gets emitted. But for many flags, we should just let them be set with as little ceremony as possible.

5 Likes

I am generally in favor of this direction, with a few caveats. I think it's important that as we lift restrictions in this area we're not closing off future opportunities to evolve the tools.

@allevato's point about the build system wanting knowledge of certain flags is a good one. Any flags which affect the inputs or outputs of compilation are particularly important when ensuring hermeticity, correctness of incremental builds, etc. I would propose:

  • SwiftPM should warn on a best-effort basis if a flag which has dedicated API (e.g. a warning control flag) is specified via unsafeFlags instead of via that API. When updating tools versions, package authors should generally move flags out of unsafeFlags if new package API allows them to do so. This gives us better tools to capture information about significant flags in structured form.

  • I think we should consider an API which allows declaring additional compile inputs and outputs corresponding to unsafe flags in the package manifest, so that package authors can maintain correct incremental builds.

3 Likes

I fully agree. This does not preclude our continuing efforts to handle build settings in a formal way in SwiftPM.

I'd actually like to see significant effort spent in rethinking how we do build settings for packages given the replatforming on Swift Build and its powerful build settings model. We probably want something more formal than xcconfig files, but somehow open up that world while ensuring a higher level developer experience.

3 Likes

I support this, while echoing and augmenting @owenv's caveats/concerns. Specifically:

Strongly agree with this one, and I think that strikes a great balance between flexibility and structured understanding of the configuration model.

This one I think is interesting. Building on this point as well as @allevato's regarding the Bazel library_evolution=True vs copts, we have many cases in Swift Build today where we parse the resolved command line of a tool invocation and infer input and output dependencies from it, for example an uncommon linker flag like -interposable_list <filename>.

We should probably continue to do more of that sort of thing, so that as SwiftPM continues to replatform onto Swift Build, more of those sort of things "just work" when the user is using unsafeFlags without the user having to do anything extra to get proper dependency ordering.

Maybe we could take a solution similar to the other part of your post I quoted, where the API to explicitly declare inputs and outputs exists, but if Swift Build already has builtin recognition for the particular flag you're using, emit a warning that the explicit dependency you've added is redundant.

Yeah, we need to ensure that we are still adding package manifest API where it makes sense to do so, and not relying on unsafeFlags as a catch-all. Even the Java example in this pitch I think could benefit from something a bit more structured, and/or we might want to add Java support to Swift Build given that it's a platform-level language on one of the platforms the Swift ecosystem is aiming to support (Android).

More than probably, I think we definitely want something more formal. If we exposed .xcconfig files directly to users via the SwiftPM project model I think it would tie packages too closely to Swift Build and (historically Xcode) implementation details and make it more difficult to evolve.

I generally don't agree with the security arguments surrounding unsafeFlags - I think ultimately the user needs to take on the responsibility of deciding whether they trust the code/dependencies they're building. We shouldn't block the entirety of possible legitimate use cases because of the possibility of misuse, when manual review is a perfectly reasonable defense against misuse and is something that any responsible developer should be doing anyways, when depending on any third party code.

For example, Visual Studio Code asks if you trust the authors when opening a workspace - allowing code execution but providing you a heads up that it's your responsibility to decide to grant that trust - to me that sort of UX is the perfect balance.

4 Likes

Given the consensus for doing something in 6.2, how to make some MVP work for all users (for all time)?

I see the benefit to unblocking some communities represented here, but worry about the volume of downstream, obscure problems caused for the broader community. The absence of package management and validation solutions for Swift means these silly strict conventions are the only protections they have.

One minimal alternative is not a change of default behavior but an opt-in mode flag at invocation time (with guards to prevent mixed-mode incremental build adoption). And for 6.2 it should be considered an experimental feature, so that it can be retracted post-evolution-discussion.

There are consequences to relaxing safety by default late in a point release. What if you decide to restore the default later? If people start to use it - swift-java, Foundation, embedded - could you ever go back on the decision? Any later evolution discussion would be strongly constrained by compatibility. If the evolution discussion added new flags or requirements, would package producers & consumers have different package files for 6.1, 6.2, and 6.3+ tooling?

An alternative to default change or wholesale invocation mode would be refined declarations for module producers and consumers. The producer could specify safeFlags[...] in the declaration, asserting the flags are safe (acknowledging they like Sendable are otherwise unvalidated); then the module consumer has a corresponding (per-dependency) option to permit such flags (for any transitive dependencies). This is more precise/clear for users, and might be a safer in the sense of impacting less code at runtime. But compatibility here would restrict the Evolution discussion even more. This could work as the 6.2 MVP only if it's clearly supportable long-term.

Stepping back a bit, these action-at-a-distance features and fixes can be pretty hard to evaluate. Before redesign, knowing what the build plan is now would be really helpful.

For versioning, results are boiled down to Package.resolved.

Could we surface some canonical form for the compiler plan for building a package and its requirements (the low-level build or SwiftPM variant of llvm's build database)? Then one could clearly evaluate before/after results for a pitch/feature like this (rather than speaking only in terms of compiler behavior).

That opens up the build plan as a design target. One can configure (xconfig?) policies for making these build plans: defaults, whitelists, modes (per intention (like a secure library), or per version (like backwards compatibility), etc. One might also have a mode that permits hand-editing of such resolved build plans, working that into the lifecycle (with flags to refresh or avoid overwriting the build plan).

Overall, if/since safety and clear modeling of Swift flags are always goals, that only increases the pressure to enable users to solve their own problems. We need a long-term way to unblock high-agency communities without exporting difficult and obscure errors to the vast community of ordinary Swift package users.

1 Like

Dependencies referenced by a git branch, version, or range have the potential to change after your initial review.

I agree we shouldn't block legitimate use cases and balance security with usability. However, manual review is most effective in environments where you can confidently trust the supply chain, verify dependency integrity, or at least confirm their provenance. When dependencies are hosted on GitHub and specified with flexible version requirements such as upToNextMajor, such verification processes become more challenging.

4 Likes

I'm incredibly happy to see this proposed for 6.2!

Without this swift-java would be unable to have a stable release in practice -- as Doug rightly explains with the depending on packages with unsafe flags. So it's absolutely on the critical path for that effort, and the general android support as well in the long run -- once we'd start making use of swift-java there as well.

Other than that, I've hit this problem in the past numerous times but usually was able to find some nasty workaround. This time it's different though since we'd those flags it in a stable release.

I agree with the proposal to just lift the restriction now without much improvement, and then revisit the entire way how we declare things as many folks have proposed here as well already. Especially since the setting already is "unsafe" it should go with the usual "we're not going to check any of this" meaning that unsafe implies in Swift -- so I'm not concerned about lifting the restriction.

In the future the ideas about warnings are great, and we could also bring back some way to "don't trust a package that uses unsafe flags" once we have sufficient replacements for some of the flags etc :slight_smile:

4 Likes

Without taking a position on the overall pitch, it would seem to me that this view of the future is...unrealistic? If current restrictions on unsafe flags haven't been a forcing function for "safe" replacements—which, of course, they haven't since the motivation here is predicated on a lack of them—it's doubtful that removing those restrictions would put us on a faster path to that future. Put simply, I can't imagine SwiftPM will ever return to re-enabling these checks once they're disabled.

3 Likes

The "don't trust a package ..." I was imagining to be opt-in if at all. Definitely not just bringing back the behavior -- if folks in the thread felt that was something they'd like to enable.

Personally I don't care for this unsafe flags preventing, though the warnings if we know there's a better way to express some of them that may be nice -- but again, not something I'm desperate for. We are in deep need of actually being able to build and release projects which require flags though.

3 Likes

I would expect TOFU to be a solution that is both user-friendly and safe in the first place. SwiftPM could prompt the user with the usage of a specific set of "unsafe flags" from a package at the first time it appears in the current checkout and ask them for explicit trust. The user should also be able to permanently trust all unsafe flags from a certain package for convenience or automation, and that trust shall not be propagated.

In theory this could be even "safer" than the current approach because it extends the protection to users that are not using versioned dependency.

7 Likes

I think this pitch is reasonable and justified.

For when we're discussing a more first class solution is SwiftPM, I guess this is an instance of a package, possibly deep in your dependency graph, needing some "parameter" to be able to resolve. Such a concept could become part of PackageDescription, and there could be a function like getRequiredParameter("JAVA_HOME") or declare parameters similarly to traits, that swift-java could use in Package.swift, and that could come from env vars, or from the end user's CLI invocation with swift build --parameter JAVA_HOME=/path/to/java. And most importantly, if it's missing, SwiftPM would provide a great message that tells you:

  • which package is missing which parameter name
  • which direct dependencies of yours are pulling in that transitive dependency that's failing
  • snippets of how to specify the parameter (env var or CLI or other ways, maybe even some file in .swiftpm)
2 Likes

Since we were just chatting about it with @Honza_Dvorsky I can add how the failure mode looks today:

Missing to have a JAVA_HOME and /usr/libexec/java_home failing to locate a java would end up in such resolve issue:

And similar in command line.

This actually highlights another issue but I don't want to pull it into this thread: the fact that this "findJavaHome()" method is pretty big and we have to copy paste it everywhere into Package.swift where you need to find a JAVA_HOME...

But that's quite separate from the topic of this pitch so I'll leave it at that and let's discuss that bit some other time when we'd like to lean into some new APIs for such "required" configuration.

3 Likes

Your review and application of trust needs to be applied to the post-package-resolution state of your code and its dependencies, e.g. what is tracked by Package.resolved. That does not have the potential to change after your review because you need to always be reviewing the resolved state in the first place.

In practical terms, SwiftPM is tightly coupled to git today, and git is a content addressable store, so as long as you're committing your Package.resolved to your repo and the hashes of dependencies have not changed, you can guarantee the content of those dependencies has not either. You can also vendor and/or mirror your dependencies from a trusted host to help verify integrity.

1 Like

Thanks everyone for the great feedback. I do acknowledge that disabling the check could lead to a degrade in the quality of the package experience if too many flags are introduced that break consumers. We will continue to study for good solutions to help mitigate that while still allowing for packages and their consumers to use the flags they need.

I have a PR up to disable the one line that does the check. Disable check for unsafe flags. by dschaefer2 · Pull Request #8896 · swiftlang/swift-package-manager · GitHub.

Thanks again!

4 Likes

I've been through the proposal and it seems like a sensible decision to me. I think the risk of degrading the package experience is relatively low and something we can iterate on. Compared to the current state where packages are unusable I'm in favour of this change!

5 Likes

We discussed this pitch in yesterday’s Ecosystem Steering Group meeting. The pitched change doesn’t have an impact on the public API of the manifest or CLI arguments, hence, it doesn’t require an evolution proposal. We felt like it was still worth discussing its impact on the broader package ecosystem. Overall, we are supportive of this change since it currently prohibits many important use-cases from shipping SemVer-compatible packages. We feel that it is important to unblock those use-cases with the goal of broadening the ecosystem outside of the currently blessed path of safe and typed settings. In the future, we would like to see many of the current unsafe flags gain a safe and typed API instead.

8 Likes