Skip to content

Use SystemPath for PacketFilter. - #1518

Merged
jglogan merged 1 commit into
apple:mainfrom
full-chaos:feat/chaos-1449-packetfilter-filepath
May 8, 2026
Merged

Use SystemPath for PacketFilter.#1518
jglogan merged 1 commit into
apple:mainfrom
full-chaos:feat/chaos-1449-packetfilter-filepath

Conversation

@chrisgeo

@chrisgeo chrisgeo commented May 7, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

URL conflates web URLs with filesystem paths and carries percent-encoding semantics that are inappropriate for filesystem operations. SystemPackage's FilePath is purpose-built for filesystem paths.

Changes in this PR:

  • configURL / anchorsURL → configPath / anchorsPath (FilePath)
  • defaultConfigPath / defaultAnchorsPath now FilePath
  • FileManager calls switch from atPath: url.path to atPath: path.string
  • Test uses String(contentsOfFile: path.string, …) in place of String(contentsOf: URL, …)

Process.executableURL = URL(fileURLWithPath: "/sbin/pfctl") is preserved as URL since Foundation.Process requires it.

Testing

  • Tested locally (swift test --filter PacketFilterTest passes; full ContainerAPIClientTests suite passes — 171 tests, 0 failures)
  • Added/updated tests (test fixtures updated to construct PacketFilter with FilePath)
  • Added/updated docs

@chrisgeo
Use SystemPath for PacketFilter.
db825ef 
Replaces URL-based filesystem operations with SystemPackage.FilePath,
mirroring the pattern landed in apple#1480 for HostDNSResolver:

- configURL/anchorsURL → configPath/anchorsPath: FilePath
- defaultConfigPath/defaultAnchorsPath now FilePath
- FileManager calls switch from atPath: url.path to atPath: path.string
- Tests use String(contentsOfFile:) in place of String(contentsOf: URL)

Process.executableURL stays URL since Foundation.Process requires it.
jglogan
jglogan approved these changes May 8, 2026
View reviewed changes

@jglogan jglogan left a comment

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.

@chrisgeo this looks great, thank you!

@jglogan
jglogan merged commit af8071b into apple:main May 8, 2026
4 checks passed
This was referenced May 8, 2026
Closed
Closed
Closed
Merged
Merged
chrisgeo added a commit to full-chaos/container that referenced this pull request May 8, 2026
@chrisgeo
Use SystemPath for ContainerResource.Filesystem.
2bfc155 
Replaces URL-based path normalization with SystemPackage.FilePath in
the three Filesystem factory methods (block, volume, virtiofs):

- source: URL(fileURLWithPath: source).absolutePath() → FilePath(source).string

All existing callers (Parser, Utility, SnapshotStore, RuntimeConfigurationTests)
already pass absolute paths; the URL round-trip was a no-op in practice.
FilePath preserves separator normalization equivalent to the URL form.

Following the migration pattern from apple#1480 (HostDNSResolver),
apple#1518 (PacketFilter), and discussion apple#1481.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 8, 2026
@chrisgeo
Use SystemPath for DirectoryWatcher.
4c1477d 
Replaces URL-based filesystem path handling with SystemPackage.FilePath,
following the pattern from apple#1480 (HostDNSResolver) and apple#1518 (PacketFilter):

- directoryURL → directoryPath: FilePath (public API)
- startWatching handler signature changes from ([URL]) to ([FilePath])
- LocalhostDNSHandler caller updated; the URL(filePath: configPath.string)
  wrapping shim is no longer needed
- FilePath.lastComponent (returns Optional<Component>) handled in
  LocalhostDNSHandler's filename filter
This was referenced May 8, 2026
Open
Open
Open
Open
jglogan pushed a commit that referenced this pull request May 11, 2026
@chrisgeo
Use SystemPath for DirectoryWatcher. (#1522)
509fa04 
- Closes #1520.
- Using URL for filesystem paths is bad practice. FilePath is safer and
more ergonomic.
- Same pattern as #1480 (HostDNSResolver) and #1518 (PacketFilter).
jglogan pushed a commit that referenced this pull request May 12, 2026
@chrisgeo
Use SystemPath for ContainerResource.Filesystem. (#1523)
940fefb 
- Closes #1521.
- Using URL for filesystem paths is bad practice. FilePath is safer and
more ergonomic.
- Same pattern as #1480 (HostDNSResolver) and #1518 (PacketFilter).
chrisgeo added a commit to full-chaos/container that referenced this pull request May 13, 2026
@chrisgeo
Use SystemPath for SandboxRuntimeConfiguration.
4c2c6b9 
Converts RuntimeConfiguration.path, runtimeConfigurationPath, and the
readRuntimeConfiguration(from:) parameter from URL to FilePath. Following
the migration pattern from apple#1480 (HostDNSResolver), apple#1518 (PacketFilter),
and discussion apple#1481.

The two server-side callers (ContainersService, SandboxService) get
minimal URL <-> FilePath bridge calls. Bundle.create still requires URL
(Foundation API) so the bridge stays at that boundary.

Bundle+Log.swift in the same module is intentionally NOT converted in
this PR: all three of its callers use Foundation FileHandle APIs that
require URL, and converting bundle.containerLog without simultaneously
updating those callers (in ContainersService and SandboxService) would
expand the PR scope significantly. Bundle+Log can be converted later as
part of a SandboxService/ContainersService cluster conversion.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 13, 2026
@chrisgeo
Use SystemPath for PluginConfig.
83e1062 
Convert PluginConfig.init from URL to FilePath, following the migration
pattern from apple#1480 (HostDNSResolver), apple#1518 (PacketFilter), and
discussion apple#1481.

- PluginConfig.init?(configURL: URL) → init?(configPath: FilePath)
- DefaultPluginFactory.findConfigURL → findConfigPath (returns FilePath?)
- Both internal call sites (DefaultPluginFactory, AppBundlePluginFactory)
  updated; bridge from URL to FilePath stays at the directory boundary
  since broader PluginFactory conversion is tracked separately.
- Tests updated to derive FilePath from temp URL.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 13, 2026
@chrisgeo
Use FilePath for SnapshotStore.
eabc49e 
- Closes CHAOS-1454.
- Internal cluster; URL boundary preserved at `directorySize` for `URLResourceKey.totalFileAllocatedSizeKey`.
- Sibling of apple#1480 (HostDNSResolver) and apple#1518 (PacketFilter), same pattern.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 13, 2026
@chrisgeo
Use SystemPath for SandboxRuntimeConfiguration.
d487c6d 
Converts RuntimeConfiguration.path, runtimeConfigurationPath, and the
readRuntimeConfiguration(from:) parameter from URL to FilePath. Following
the migration pattern from apple#1480 (HostDNSResolver), apple#1518 (PacketFilter),
and discussion apple#1481.

The two server-side callers (ContainersService, SandboxService) get
minimal URL <-> FilePath bridge calls. Bundle.create still requires URL
(Foundation API) so the bridge stays at that boundary.

Bundle+Log.swift in the same module is intentionally NOT converted in
this PR: all three of its callers use Foundation FileHandle APIs that
require URL, and converting bundle.containerLog without simultaneously
updating those callers (in ContainersService and SandboxService) would
expand the PR scope significantly. Bundle+Log can be converted later as
part of a SandboxService/ContainersService cluster conversion.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 13, 2026
@chrisgeo
Use SystemPath for PluginConfig.
07c10a8 
Convert PluginConfig.init from URL to FilePath, following the migration
pattern from apple#1480 (HostDNSResolver), apple#1518 (PacketFilter), and
discussion apple#1481.

- PluginConfig.init?(configURL: URL) → init?(configPath: FilePath)
- DefaultPluginFactory.findConfigURL → findConfigPath (returns FilePath?)
- Both internal call sites (DefaultPluginFactory, AppBundlePluginFactory)
  updated; bridge from URL to FilePath stays at the directory boundary
  since broader PluginFactory conversion is tracked separately.
- Tests updated to derive FilePath from temp URL.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 14, 2026
@chrisgeo
Use FilePath for SnapshotStore.
8a469ca 
- Closes CHAOS-1454.
- Internal cluster; URL boundary preserved at `directorySize` for `URLResourceKey.totalFileAllocatedSizeKey`.
- Sibling of apple#1480 (HostDNSResolver) and apple#1518 (PacketFilter), same pattern.
This was referenced May 23, 2026
Closed
Merged
chrisgeo added a commit to full-chaos/container that referenced this pull request May 24, 2026
@chrisgeo
Use SystemPath for PluginConfig.
3bdfaab 
Convert PluginConfig.init from URL to FilePath, following the migration
pattern from apple#1480 (HostDNSResolver), apple#1518 (PacketFilter), and
discussion apple#1481.

- PluginConfig.init?(configURL: URL) → init?(configPath: FilePath)
- DefaultPluginFactory.findConfigURL → findConfigPath (returns FilePath?)
- Both internal call sites (DefaultPluginFactory, AppBundlePluginFactory)
  updated; bridge from URL to FilePath stays at the directory boundary
  since broader PluginFactory conversion is tracked separately.
- Tests updated to derive FilePath from temp URL.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 24, 2026
@chrisgeo
Use FilePath for SnapshotStore.
8e99522 
- Closes CHAOS-1454.
- Internal cluster; URL boundary preserved at `directorySize` for `URLResourceKey.totalFileAllocatedSizeKey`.
- Sibling of apple#1480 (HostDNSResolver) and apple#1518 (PacketFilter), same pattern.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 24, 2026
@chrisgeo
Use FilePath for CLI image/container path-arg parsing.
fe910bb 
- Closes CHAOS-1457.
- Adds shared FilePath.absolute(_:relativeTo:) helper.
- Sibling of apple#1480 (HostDNSResolver) and apple#1518 (PacketFilter), same pattern.
chrisgeo added a commit to full-chaos/container that referenced this pull request May 24, 2026
@chrisgeo
Use SystemPath for SandboxRuntimeConfiguration.
0106155 
Converts RuntimeConfiguration.path, runtimeConfigurationPath, and the
readRuntimeConfiguration(from:) parameter from URL to FilePath. Following
the migration pattern from apple#1480 (HostDNSResolver), apple#1518 (PacketFilter),
and discussion apple#1481.

The two server-side callers (ContainersService, SandboxService) get
minimal URL <-> FilePath bridge calls. Bundle.create still requires URL
(Foundation API) so the bridge stays at that boundary.

Bundle+Log.swift in the same module is intentionally NOT converted in
this PR: all three of its callers use Foundation FileHandle APIs that
require URL, and converting bundle.containerLog without simultaneously
updating those callers (in ContainersService and SandboxService) would
expand the PR scope significantly. Bundle+Log can be converted later as
part of a SandboxService/ContainersService cluster conversion.
@BrewTestBot BrewTestBot mentioned this pull request Jun 9, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jglogan jglogan jglogan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Request]: TECH DEBT - Convert PacketFilter to use FilePath instead of URL.

2 participants

@chrisgeo @jglogan