Conversation
7e1e0ba to
cc054da
Compare
The decrypted `SPCredentials.swift` no longer lands in the checkout: each consumer target's build phase declares its own `$(DERIVED_FILE_DIR)` output and the script writes there, via `SCRIPT_OUTPUT_FILE_0`. Part of AINFRA-2731, applying the pattern doapple and woocommerce-ios developed and Gravatar-SDK-iOS solidified. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 4.8 <noreply@anthropic.com>
Replaces the `SimplenoteSecrets` aggregate target — which wrote the plaintext into `Simplenote/Credentials/` — with a `Copy Secret` phase on each target that actually compiles the file, writing into that target's own `$(DERIVED_FILE_DIR)`. The two file references are `DERIVED_FILE_DIR`-relative, so Xcode shows them in red with no resolvable path until a build runs. `Simplenote/DerivedSources/README.md` exists to stop the next reader "fixing" that. `Simplenote/Credentials/` stays gitignored so a stale decrypted copy from before this change cannot be committed. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 4.8 <noreply@anthropic.com>
The build phase now falls back to the demo credentials on its own, so copying them into place by hand is both unnecessary and, since the destination moved out of the checkout, wrong. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 4.8 <noreply@anthropic.com>
Matches `Scripts/build-phases/generate-secrets.sh` in Gravatar-SDK-iOS, whose DerivedData secret generation this branch is modelled on. Style only: `#!/bin/bash -euo pipefail` does apply all three options on macOS. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
`$(dirname …)` and the `cmp` operands were unquoted, so a `DERIVED_FILE_DIR` containing a space — an Xcode path the script does not control — split into multiple words. Verified against a destination under `dir with space/`. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
Mirrors the `apply()` helper in Gravatar-SDK-iOS's `generate-secrets.sh`. The build log now names the source path instead of labelling it "Production" or "Example"; the following commit drops the `DerivedSources/README.md` pointer to those old strings. Keeps `cp -v`, which Gravatar does not need: with two consumer targets writing to different `DERIVED_FILE_DIR`s, the destination in the log is what tells you which target's copy you are looking at. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
Matches the length and scope of `Demo/Demo/Generated/README.md` in Gravatar-SDK-iOS: say why the reference is red, say not to delete it, point at the script. The dropped material — why `DERIVED_FILE_DIR` cannot resolve at parse time, a sample Derived Data tree, how to reveal a generated file in Finder — explained Xcode rather than this project. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
cc054da to
bee3771
Compare
|
|
||
| # Settings | ||
| Simplenote/config.plist | ||
| Simplenote/Credentials/ |
There was a problem hiding this comment.
I considered keeping this just to avoid leaking. But given the repo is in maintenance, meaning it doesn't see a lot of active development, the chance of a leak are low and I prioritized keeping things tidy.
Additionally, worth remembering, that even if something from SPCredentials.swift leaked, its content are already de facto public because they are part of the shipped binary.
There was a problem hiding this comment.
I get it, but at the same time, retaining one line (perhaps adding a comment) is cheap 🙂 anyway not a big deal
This mirrors the agreed implementation in Gravatar SDK iOS. See Automattic/Gravatar-SDK-iOS#824
| # 1. ${SECRETS_ROOT}, for internal contributors. `bundle exec fastlane run | ||
| # configure_apply` decrypts it there, outside the repo; this phase only | ||
| # reads it. | ||
| # 2. Simplenote/SPCredentials.external-contributors.swift — gitignored, so | ||
| # external contributors can keep their own Simperium credentials with | ||
| # little-to-no risk of committing them, starting from a copy of the | ||
| # committed template. | ||
| # | ||
| # If neither is present, the build will fail. | ||
|
|
||
| SECRETS_ROOT="${HOME}/.configure/simplenote-macos/secrets" | ||
| SECRETS_FILE="${SECRETS_ROOT}/SPCredentials.swift" |
There was a problem hiding this comment.
Of course, the paths here will soon change as we adopt a8c-secrects.
The `test` lane never ran `configure_apply`, unlike `test_app_store_build`, so this job only ever built against the demo credentials the build phase fell back to. Removing that fallback left it with no credentials at all. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
Replaces the `configure_apply` added a commit ago. The unit tests never needed real credentials — the removed demo fallback was supplying them — so copying the template keeps this job out of the secret store entirely, and turns it into a regression test for the flow the readme gives external contributors. `verify-app-store-target-builds` still decrypts, so the internal path stays covered. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
Xcode already skips the phase when neither the declared inputs nor the outputs changed, so `cmp` only ever runs in the cases where the copy is warranted. Same conclusion as Gravatar-SDK-iOS reached for `generate-secrets.sh`. Part of AINFRA-2731. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR moves secret materialization out of the repository checkout by generating SPCredentials.swift into each target’s $(DERIVED_FILE_DIR) via per-target “Copy Secret” build phases, preparing the project to later adopt a8c-secrets.
Changes:
- Replace the aggregate “SimplenoteSecrets” flow with per-target “Copy Secret” build phases that write
SPCredentials.swiftinto$(DERIVED_FILE_DIR). - Replace demo credentials with a committed
SPCredentials.template.swiftand a gitignoredSPCredentials.external-contributors.swiftfallback. - Update CI and repo docs/ignore rules to support the new derived-sources workflow.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Simplenote/SPCredentials.template.swift | Updates header comment to a credentials template intended for external contributors. |
| Simplenote/DerivedSources/README.md | Documents the intentionally-red derived source references and how they’re generated. |
| Simplenote.xcodeproj/project.pbxproj | Removes aggregate secrets target; adds derived file references and per-target “Copy Secret” phases outputting to $(DERIVED_FILE_DIR). |
| Scripts/Build-Phases/copy-secret.xcfilelist | Updates declared inputs to include the gitignored external-contributor credentials file. |
| Scripts/Build-Phases/copy-secret.sh | Writes secrets to SCRIPT_OUTPUT_FILE_0 (derived dir), adds clearer failure messaging, and updates input/output guarding. |
| readme.md | Updates credential setup narrative to describe generated credentials rather than copying demo values. |
| .gitignore | Ignores derived sources except the explanatory README; ignores external contributor credentials file. |
| .buildkite/commands/build-and-test.sh | Ensures CI unit tests have a credentials source by copying the template into the gitignored file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There are none to distribute since the demo credentials came out: internal contributors decrypt their own, external ones supply their own. #1269 (comment) --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
Without it, a run outside Xcode aborts on an unbound variable instead of printing the error the guard exists to print. #1269 (comment) --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Shared demo credentials remain and legacy plaintext credentials can become unignored.
Review details
Suppressed comments (1)
Simplenote/SPCredentials.template.swift:4
- The template still contains the former demo Simperium app ID and API key, so copying it already supplies the shared credentials without requiring contributors to “fill in” their own. This contradicts the PR’s removal of shared demo credentials and also makes CI continue compiling with them; replace these values with empty placeholders.
- Files reviewed: 7/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
| ``` | ||
|
|
||
| This will allow you to compile and run the app on a device or a simulator. | ||
| Credentials live in the `simperium*` properties of the `SPCredentials` type, which is generated at build time from production credentials kept outside the checkout, falling back to user-specified ones. |
There was a problem hiding this comment.
Maybe good to include the template copy command here and explain what works without supplying your own credentials? 🤔
What? 😄 |
iangmaia
left a comment
There was a problem hiding this comment.
Looks good, but I'll leave the final merge to you (I left a couple of nits).
Prep step to then adopt
a8c-secrets, which requires secrets outside of the repo. See https://linear.app/a8c/issue/AINFRA-2731.I modelled the script changes on the recent Gravatar SDK implementation, which benefitted from the back and forth with @AliSoftware.
Same
Secrets.template.swift→*.external-contributors.swiftpair, writing toSCRIPT_OUTPUT_FILE_0underDERIVED_FILE_DIRREADMEexplaining sources generation and why they are redDifferent, aside for the differences due to the repo not using
a8c-secretsyetSimplenoteandIntentsExtension. TheSCRIPT_OUTPUT_FILE_0indirection is "load-bearing" here, because so one script and one.xcfilelistback two identical phases, each writing into its own target's derived dirensure_is_in_input_files_listcheck (errors if a source isn't declared in the.xcfilelist). Gravatar hasn't one, but I think it's useful and we should eventually adopt it there, too (especially if the endgame is for the script to be generic enough that it can run froma8c-secrets, i.e. without the source being in the repo and therefore the only knobs we can turn being input and ouput paths/lists)AI-generated Details
Fix
The decrypted
SPCredentials.swiftno longer lands in the checkout.Each target that compiles it —
SimplenoteandIntentsExtension— carries aCopy Secretbuild phase writing into that target's own$(DERIVED_FILE_DIR), replacing theSimplenoteSecretsaggregate target that wrote intoSimplenote/Credentials/.Internal contributors no longer copy anything into place by hand.
The shared demo credentials are gone with it.
SPCredentials-demo.swiftbecomesSPCredentials.template.swift, which external contributors copy to the gitignoredSPCredentials.external-contributors.swiftand fill in with their own Simperium credentials.With neither that file nor the decrypted secrets present, the build fails in every configuration rather than silently compiling against demo values.
Part of AINFRA-2731, applying the pattern Gravatar-SDK-iOS settled on.
The commit messages carry the per-step rationale.
Test
Both consumer targets should compile, each phase writing to its own
…/<Target>.build/DerivedSources/SPCredentials.swift.CI exercises both sources:
build-and-testcopies the template, so it runs the flow the readme gives external contributors, whileverify-app-store-target-buildsdecrypts and builds against the real secrets.Review
One developer is enough.
The two
SPCredentials.swiftentries show in red in Xcode with no resolvable path.That is expected —
DERIVED_FILE_DIRonly resolves while a specific target is building — andSimplenote/DerivedSources/README.mdexists so the next reader does not "fix" it.