Skip to content

fix(config): persist geo-filter saves through symlinked config paths #407

Closed

Description

@dborup

Context

Assessment baseline: dborup/CoreScope master 4a5ec5e231dd97ed7a4392e50e5f409f4dc5fab0 (13 October 2026). Recheck current master and competing work before implementation.

The Go entrypoint can expose /app/config.json as a symlink to the persistent data directory. SaveGeoFilter currently renames over the symlink pathname instead of replacing its target. A save may appear successful while the persistent mounted config remains unchanged, so the saved filter can disappear when the container is recreated.

Source for later comparison

Verified from GitHub's PR metadata and complete changed-file listing on 13 October 2026. The identifiers below are literal text, not hyperlinks or cross-repository references.

Source repository: Kpa-clawbot/CoreScope
Pull request number: 2137
PR head commit SHA: 52ee01ccf9911b433ab49eb300c68c7d1fdc652b
Upstream merged SHA (merge_commit_sha): 478f74a452b5d3a9764da7f405defdd5cd592e8b
Files changed in this PR:
  - cmd/server/config.go
  - cmd/server/config_owner_unix.go
  - cmd/server/config_owner_unix_test.go
  - cmd/server/config_owner_windows.go
  - cmd/server/config_symlink_test.go

These are historical upstream comparison sources, not proof of equivalence with our fork and not a request to cherry-pick blindly. Recheck our current implementation and preserve fork-specific behavior.

Proposed work

  • Resolve and validate the effective target without replacing a valid config symlink.
  • Preserve this fork's unique temporary file, restrictive permissions, ownership and sync/error handling rather than replacing the entire save implementation.
  • Handle dangling links, chained/relative links, read-only mounts and atomic-rename failures explicitly; never report a successful persistence when it did not occur.
  • Coordinate with the opt-in non-root work in security(docker): opt-in non-root Go services with safe volume ownership (#339) #404 and preserve deliberate permission restrictions.
  • Add filesystem regression tests that inspect the target contents and verify the symlink remains intact; include a persistence/reopen test.

Acceptance criteria

A successful save updates the persistent target while preserving the symlink and existing safety invariants. Unsupported/read-only cases remain explicit errors, not permission bypasses.

Existing work / dependencies

Related internal work: #340 and #404. This is distinct from preserving file mode.

Scope

This issue records evaluation/follow-up work in our fork. It does not authorize deployment, production/staging changes, mail delivery, automatic key promotion or other external actions. Preserve fork-specific correctness, bounded work and the read-only measurement-server database contract; any separate account database exception requires explicit review.

Source performance numbers are not measurements of our deployment. Any claimed improvement requires our own benchmark and an independently reviewed implementation.

Activity

  1. dborup commented on Oct 13, 2026

    @dborup
    OwnerAuthor

    Fixed by #422 (merged as d34b228a88bc3b17e6602d3b9d34ad8c453f0a2a). Remaining nits from the review on #422: the EvalSymlinks error branch is not pinned by a test, and read-only mounts with a writable directory are not detected (documented).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions