Feature/#12 - SwiftData 기반 Secret CRUD 및 보안 계층 구성 - #16
Merged
Conversation
9 tasks
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (63)
WalkthroughSecret 도메인 계약, payload/metadata 타입, SwiftData·Keychain 저장 계층, CRUD 저장소, 유스케이스, 데모 UI, 앱 진입점이 추가되었습니다. ChangesSecret 데이터 생명주기 관리
Sequence Diagram(s)sequenceDiagram
participant DevaultApp
participant LocalStorage
participant ContentView
participant SecretUseCaseDemoView
participant SecretRepositoryImpl
participant SecretCryptoServiceImpl
participant LocalUserAuthenticationServiceImpl
DevaultApp->>LocalStorage: makeDefault()
DevaultApp-->>ContentView: storage
ContentView->>SecretUseCaseDemoView: inject use cases
SecretUseCaseDemoView->>SecretRepositoryImpl: fetch / create / patch / delete
SecretUseCaseDemoView->>SecretCryptoServiceImpl: encryptPayload / decryptPayload
SecretUseCaseDemoView->>LocalUserAuthenticationServiceImpl: authenticate(reason)
Possibly related PRs
Suggested labels
Suggested reviewers
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
Projects/DVDomain/Sources/Entity/Secret.swift (1)
14-14: 🏗️ Heavy liftBool 프로퍼티 네이밍을
isLiked로 통일하세요.Line [14]의
liked는 Bool 네이밍 규칙과 어긋납니다.isLiked로 바꾸면 도메인 의도가 더 명확하고 호출부 가독성이 좋아집니다(동일 필드가 있는 Draft/Patch도 함께 정리 권장).As per coding guidelines, 'Bool 프로퍼티에
is,has,can등의 접두사가 있는지 확인하세요.'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVDomain/Sources/Entity/Secret.swift` at line 14, Secret 구조체의 Bool 프로퍼티명 `liked`는 네이밍 규칙에 어긋나므로 `isLiked`로 리네임하세요; 변경해야 할 곳은 Entity/Secret의 선언(`public var liked` → `public var isLiked`), 관련 Draft/Patch 타입들(동일 필드명), 초기화/복사 메서드, Codable CodingKeys, DB/mapping 키, 사용처(모든 참조와 테스트), 그리고 Equatable/Hashable 구현부입니다; 리네임 시 기존 외부 API/직렬화 포맷이 변경되는 경우 기존 키와 매핑을 유지하거나 마이그레이션 처리(예: CodingKeys에 이전 이름 포함)하도록 함께 조정하세요.Projects/DVDomain/Sources/Repository/SecretQuery.swift (1)
31-47: ⚡ Quick win
public extension대신 선언별 접근 제어로 바꾸세요.Line [31]의
public extension SecretQuery는 현재 저장소 규칙과 충돌합니다.Collection,Sort를public enum으로 직접 선언하는 형태로 바꾸는 게 맞습니다.변경 예시
-public extension SecretQuery { - enum Collection: Equatable, Sendable { +extension SecretQuery { + public enum Collection: Equatable, Sendable { case all case liked case expired(referenceDate: Date) case deleted case project(id: UUID) } - enum Sort: Equatable, Sendable { + public enum Sort: Equatable, Sendable { case recentlyAdded case oldestFirst case expiringSoon case nameAscending case nameDescending } }As per coding guidelines, '
public extension패턴 대신 각 선언에 직접 접근 제어가 명시되어 있는지 확인하세요.'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVDomain/Sources/Repository/SecretQuery.swift` around lines 31 - 47, Replace the `public extension SecretQuery` block by declaring the enums with explicit access control instead of using an extension: remove the extension wrapper and make two top-level declarations `public enum Collection: Equatable, Sendable { ... }` and `public enum Sort: Equatable, Sendable { ... }` (keeping the same cases), then update any uses of `SecretQuery.Collection` / `SecretQuery.Sort` to the new types or adjust placement so the enums remain nested but declared directly inside the `SecretQuery` type with `public` access on each enum declaration rather than using `public extension`.Projects/DVDomain/Sources/UseCase/Error/SecretUseCaseError.swift (1)
12-12: ⚡ Quick win알 수 없는 오류의 원인 정보가 소실됩니다
현재
.unexpected로만 매핑되어 원인 추적이 어렵습니다. 최소한 디버깅 가능한 문자열 컨텍스트는 보존하는 편이 좋습니다.개선 예시 diff
public enum SecretUseCaseError: Error, Equatable, Sendable { @@ - case unexpected + case unexpected(message: String) } @@ - return .unexpected + return .unexpected(message: String(describing: error)) } }Also applies to: 36-36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVDomain/Sources/UseCase/Error/SecretUseCaseError.swift` at line 12, The .unexpected enum variant currently loses cause info; change it to carry context (e.g. change `case unexpected` to `case unexpected(String)` or `case unexpected(Error)` in SecretUseCaseError, update all places that construct `.unexpected` (and the other identical occurrence referenced) to pass a descriptive string such as `String(describing: error)` or `error.localizedDescription`, and update any switch/throw sites and tests to handle the associated value so the original error/debug context is preserved for logging and debugging.Projects/DVDomain/Sources/SecretContent/Metadata/DatabaseMetadata.swift (1)
10-23: ⚡ Quick winBool 프로퍼티 네이밍 접두사를 규칙에 맞춰주세요.
Line 10의
sslRequired는 Bool 타입인데is/has/can접두사 규칙과 맞지 않습니다.isSSLRequired로 통일하면 도메인 모델 전반의 가독성과 일관성이 좋아집니다.변경 제안
public struct DatabaseMetadata: SecretMetadataContent, Equatable { public static let schemaVersion = 1 public var host: String? public var port: Int? public var databaseName: String? public var username: String? - public var sslRequired: Bool? + public var isSSLRequired: Bool? public init( host: String? = nil, port: Int? = nil, databaseName: String? = nil, username: String? = nil, - sslRequired: Bool? = nil + isSSLRequired: Bool? = nil ) { self.host = host self.port = port self.databaseName = databaseName self.username = username - self.sslRequired = sslRequired + self.isSSLRequired = isSSLRequired } }As per coding guidelines, "Bool 프로퍼티에
is,has,can등의 접두사가 있는지 확인하세요."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVDomain/Sources/SecretContent/Metadata/DatabaseMetadata.swift` around lines 10 - 23, Rename the Bool property sslRequired to isSSLRequired and update its initializer parameter and assignment in the DatabaseMetadata init so the stored property and constructor parameter match (replace occurrences of sslRequired with isSSLRequired in the property declaration, initializer parameter list, and self assignment). Also search for and update all references/usages of DatabaseMetadata.sslRequired to DatabaseMetadata.isSSLRequired (including any decoding/encoding keys or Codable conformance if present) to keep the API and serialization consistent.Projects/DVData/Sources/RepositoryImpl/Secret/SecretFetchDescriptorBuilder.swift (1)
42-50: 💤 Low value강제 언래핑(
!) 사용 -#Predicate제약 확인 필요Line 46에서
secret.expiresAt!를 사용하고 있습니다. 직전 조건(secret.expiresAt != nil)으로 nil이 아님을 보장하지만,#Predicate매크로 내부에서는if let/guard let패턴을 사용할 수 없는 제약이 있습니다.SwiftData의
flatMap기반 옵셔널 비교가 가능한지 확인해 주세요. 불가능하다면 현재 패턴이 불가피하므로 주석으로 의도를 명시하는 것을 권장합니다.💡 주석 추가 제안
case let .expired(referenceDate): + // Note: `#Predicate` 내부에서는 optional binding 불가. nil 체크 후 강제 언래핑 사용. return `#Predicate`<SwiftDataModel.Secret> { secret in secret.deletedAt == nil && secret.expiresAt != nil && secret.expiresAt! < referenceDate &&🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVData/Sources/RepositoryImpl/Secret/SecretFetchDescriptorBuilder.swift` around lines 42 - 50, The force-unwrapping of secret.expiresAt inside the `#Predicate` in the .expired(referenceDate) case is unsafe given macro constraints; replace secret.expiresAt! < referenceDate with an optional-safe expression such as secret.expiresAt.map { $0 < referenceDate } == true (or, if the macro allows, (secret.expiresAt ?? Date.distantPast) < referenceDate) so no force unwrap is used; if neither optional-mapping approach is supported by `#Predicate`, retain the current logic but add a concise comment in SecretFetchDescriptorBuilder near the .expired(referenceDate) case and the `#Predicate`<SwiftDataModel.Secret> explaining why force-unwrapping is safe (because of the preceding secret.expiresAt != nil check) and noting the macro limitation.Projects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swift (1)
23-76: ⚡ Quick win중복된 Secret 조립 로직을 한 곳으로 모아 주세요.
두
execute오버로드가 동일한 필드 매핑을 반복하고 있어, 필드 추가/수정 시 한쪽만 누락될 위험이 있습니다. Secret 생성 부분을private헬퍼로 추출해 중복을 제거하는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swift` around lines 23 - 76, Extract the duplicated Secret construction in both execute overloads of CreateSecretUseCaseImpl into a private helper (e.g., makeSecret(from:draft:payload:metadataOpt:now:)) that accepts the SecretDraft, current date (from dateProvider()), encrypted payload (from cryptoService.encryptPayload), optional encoded metadata (from cryptoService.encodeMetadata), and uses idGenerator(), draft.name/secretType/subType/service/environment/expiresAt/memo/liked to set fields plus createdAt/updatedAt/payload and metadata when present; update both public execute methods to call SecretUseCaseHelper.validateDraft, compute now/encryptedPayload/(encodedMetadata), call the new helper to get the Secret, then return try await repository.create(secret), preserving existing error handling (SecretUseCaseError.map).Projects/DVPresentation/Sources/SecretUseCaseDemoView.swift (2)
54-55: ⚡ Quick win이벤트 핸들러 함수명은 과거형으로 통일해주세요.
버튼 액션 핸들러(
createDemoSecret,revealSelectedPayload,fetchAllSecrets)를didTap...형태로 맞추면 규칙 일관성이 좋아집니다.As per coding guidelines
이벤트 핸들러 함수명이 과거형인지 확인하세요. (didTapButton ✅ / handleButtonTap ❌).Also applies to: 62-63, 121-122, 171-171, 197-197, 206-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVPresentation/Sources/SecretUseCaseDemoView.swift` around lines 54 - 55, Rename the event-handler functions to past-tense `didTap...` form for consistency: change `createDemoSecret` → `didTapCreateDemoSecret`, `revealSelectedPayload` → `didTapRevealSelectedPayload`, `fetchAllSecrets` → `didTapFetchAllSecrets` (and any other handlers noted at the same locations) and update all call sites (e.g., Task { await createDemoSecret() } to Task { await didTapCreateDemoSecret() }) plus any references in UI button labels, method declarations, and tests; ensure async/throws signatures remain unchanged and run a build to fix any broken imports/usages.
3-4: ⚡ Quick win임포트 순서를 가이드에 맞춰 정리해주세요.
현재는 내장 프레임워크(
SwiftUI)가 뒤에 와 있습니다.🔧 제안 패치
-import DVDomain -import SwiftUI +import SwiftUI + +import DVDomainAs per coding guidelines
모듈 임포트가 알파벳 순으로 정렬되어 있는지 확인하세요. (내장 프레임워크 먼저, 빈 줄로 서드파티 구분).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVPresentation/Sources/SecretUseCaseDemoView.swift` around lines 3 - 4, The import statements are out of order: move the system framework import SwiftUI to be first, then add a blank line and place the module import DVDomain after it so imports follow the guideline (system frameworks first, blank line, third-party/local modules) — update the import block containing SwiftUI and DVDomain accordingly.Projects/Devault/Sources/ContentView.swift (1)
1-4: ⚡ Quick win임포트 순서를 내장 프레임워크 우선으로 정리해주세요.
🔧 제안 패치
-import DVData -import DVDomain -import DVPresentation import SwiftUI + +import DVData +import DVDomain +import DVPresentationAs per coding guidelines
모듈 임포트가 알파벳 순으로 정렬되어 있는지 확인하세요. (내장 프레임워크 먼저, 빈 줄로 서드파티 구분).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/Devault/Sources/ContentView.swift` around lines 1 - 4, Reorder the import statements so built-in frameworks come first and third-party/internal modules follow separated by a blank line; specifically move import SwiftUI to the top, then add a blank line and sort the remaining module imports alphabetically (import DVData, import DVDomain, import DVPresentation) to comply with the project’s import ordering rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Projects/DVData/Sources/ServiceImpl/Security/Keychain/KeychainKeyStore.swift`:
- Around line 102-104: Remove the force-unwrap of baseAddress in the
SecRandomCopyBytes call inside bytes.withUnsafeMutableBytes: replace the direct
$0.baseAddress! usage with a safe guard (e.g., inside the closure do guard let
base = $0.baseAddress else { return errSecParam } and pass base to
SecRandomCopyBytes) so that bytes.withUnsafeMutableBytes { ... } returns
errSecParam when baseAddress is nil; update the surrounding code in
KeychainKeyStore.swift (the bytes.withUnsafeMutableBytes / SecRandomCopyBytes
invocation) to handle the returned status accordingly.
In `@Projects/DVData/Sources/Storage/Local/LocalStorage.swift`:
- Line 10: modelContainer이 현재 `public lazy var modelContainer`로 외부에서 재할당 가능한
상태이므로 접근을 제한하세요; 수정 방법은 두 가지 중 하나로 선택합니다: 1) 인스턴스 생성 시 즉시 초기화할 수 있으면 `public let
modelContainer`로 변경하고 초기값을 생성자(init)에서 설정하거나, 2) lazy 초기화가 필요하면 `public
private(set) lazy var modelContainer`로 변경해 외부 쓰기를 막고 내부에서만 변경 가능하게 만드세요; 대상 심볼은
modelContainer이며 변경은 이 프로퍼티 선언부와 필요한 초기화 로직(초기화 생성자 또는 기존 lazy 블록)만 수정하면 됩니다.
- Around line 10-21: The lazy modelContainer currently calls fatalError on
ModelContainer init failure; change this to propagate the error instead: remove
fatalError from the modelContainer closure and refactor so the ModelContainer
creation can throw and be handled by the composition root (e.g., make
LocalStorage initializer throw or expose a throwing factory method that
constructs ModelContainer using ModelConfiguration and Schema.appSchema), or
make modelContainer optional and surface the init error via a thrown error or
result type; also tighten access control where possible (e.g., mark
modelContainer private or fileprivate if not used externally) and ensure callers
handle initialization failures to present fallback or error UI instead of
crashing.
In `@Projects/DVDomain/Sources/UseCase/Impl/Secret/FetchSecretUseCaseImpl.swift`:
- Around line 41-45: 현재 revealPayload 구현은 repository.fetch(id:)로 시크릿 존재를 먼저 확인해
SecretUseCaseError.secretNotFound를 던지므로, 인증 전에도 ID 존재 여부가 노출됩니다; authenticate를
먼저 호출하도록 revealPayload의 흐름을 변경해 authenticationService.authenticate(reason:)를 호출한
뒤 repository.fetch(id:)로 시크릿을 조회하고, 존재하지 않으면 SecretUseCaseError.secretNotFound를
던지며 마지막으로 cryptoService.decryptPayload(_:as:)로 복호화 결과를 반환하도록 수정하세요.
In `@Projects/DVPresentation/Sources/SecretUseCaseDemoView.swift`:
- Around line 197-202: In revealSelectedPayload(), the code always calls
fetchSecretUseCase.revealPayload(id:as:) with APIKeyPayload.self which causes
failures when the selectedSecret is not an API key; change the logic to inspect
selectedSecret (e.g., selectedSecret?.type or secretType) and branch by type
(switch or if) to call revealPayload with the matching payload type (or handle
unsupported types), then assign revealedPayload and statusMessage accordingly so
non-APIKey secrets don’t try to decode as APIKeyPayload.self.
---
Nitpick comments:
In `@Projects/Devault/Sources/ContentView.swift`:
- Around line 1-4: Reorder the import statements so built-in frameworks come
first and third-party/internal modules follow separated by a blank line;
specifically move import SwiftUI to the top, then add a blank line and sort the
remaining module imports alphabetically (import DVData, import DVDomain, import
DVPresentation) to comply with the project’s import ordering rule.
In
`@Projects/DVData/Sources/RepositoryImpl/Secret/SecretFetchDescriptorBuilder.swift`:
- Around line 42-50: The force-unwrapping of secret.expiresAt inside the
`#Predicate` in the .expired(referenceDate) case is unsafe given macro
constraints; replace secret.expiresAt! < referenceDate with an optional-safe
expression such as secret.expiresAt.map { $0 < referenceDate } == true (or, if
the macro allows, (secret.expiresAt ?? Date.distantPast) < referenceDate) so no
force unwrap is used; if neither optional-mapping approach is supported by
`#Predicate`, retain the current logic but add a concise comment in
SecretFetchDescriptorBuilder near the .expired(referenceDate) case and the
`#Predicate`<SwiftDataModel.Secret> explaining why force-unwrapping is safe
(because of the preceding secret.expiresAt != nil check) and noting the macro
limitation.
In `@Projects/DVDomain/Sources/Entity/Secret.swift`:
- Line 14: Secret 구조체의 Bool 프로퍼티명 `liked`는 네이밍 규칙에 어긋나므로 `isLiked`로 리네임하세요; 변경해야
할 곳은 Entity/Secret의 선언(`public var liked` → `public var isLiked`), 관련
Draft/Patch 타입들(동일 필드명), 초기화/복사 메서드, Codable CodingKeys, DB/mapping 키, 사용처(모든
참조와 테스트), 그리고 Equatable/Hashable 구현부입니다; 리네임 시 기존 외부 API/직렬화 포맷이 변경되는 경우 기존 키와
매핑을 유지하거나 마이그레이션 처리(예: CodingKeys에 이전 이름 포함)하도록 함께 조정하세요.
In `@Projects/DVDomain/Sources/Repository/SecretQuery.swift`:
- Around line 31-47: Replace the `public extension SecretQuery` block by
declaring the enums with explicit access control instead of using an extension:
remove the extension wrapper and make two top-level declarations `public enum
Collection: Equatable, Sendable { ... }` and `public enum Sort: Equatable,
Sendable { ... }` (keeping the same cases), then update any uses of
`SecretQuery.Collection` / `SecretQuery.Sort` to the new types or adjust
placement so the enums remain nested but declared directly inside the
`SecretQuery` type with `public` access on each enum declaration rather than
using `public extension`.
In `@Projects/DVDomain/Sources/SecretContent/Metadata/DatabaseMetadata.swift`:
- Around line 10-23: Rename the Bool property sslRequired to isSSLRequired and
update its initializer parameter and assignment in the DatabaseMetadata init so
the stored property and constructor parameter match (replace occurrences of
sslRequired with isSSLRequired in the property declaration, initializer
parameter list, and self assignment). Also search for and update all
references/usages of DatabaseMetadata.sslRequired to
DatabaseMetadata.isSSLRequired (including any decoding/encoding keys or Codable
conformance if present) to keep the API and serialization consistent.
In `@Projects/DVDomain/Sources/UseCase/Error/SecretUseCaseError.swift`:
- Line 12: The .unexpected enum variant currently loses cause info; change it to
carry context (e.g. change `case unexpected` to `case unexpected(String)` or
`case unexpected(Error)` in SecretUseCaseError, update all places that construct
`.unexpected` (and the other identical occurrence referenced) to pass a
descriptive string such as `String(describing: error)` or
`error.localizedDescription`, and update any switch/throw sites and tests to
handle the associated value so the original error/debug context is preserved for
logging and debugging.
In `@Projects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swift`:
- Around line 23-76: Extract the duplicated Secret construction in both execute
overloads of CreateSecretUseCaseImpl into a private helper (e.g.,
makeSecret(from:draft:payload:metadataOpt:now:)) that accepts the SecretDraft,
current date (from dateProvider()), encrypted payload (from
cryptoService.encryptPayload), optional encoded metadata (from
cryptoService.encodeMetadata), and uses idGenerator(),
draft.name/secretType/subType/service/environment/expiresAt/memo/liked to set
fields plus createdAt/updatedAt/payload and metadata when present; update both
public execute methods to call SecretUseCaseHelper.validateDraft, compute
now/encryptedPayload/(encodedMetadata), call the new helper to get the Secret,
then return try await repository.create(secret), preserving existing error
handling (SecretUseCaseError.map).
In `@Projects/DVPresentation/Sources/SecretUseCaseDemoView.swift`:
- Around line 54-55: Rename the event-handler functions to past-tense
`didTap...` form for consistency: change `createDemoSecret` →
`didTapCreateDemoSecret`, `revealSelectedPayload` →
`didTapRevealSelectedPayload`, `fetchAllSecrets` → `didTapFetchAllSecrets` (and
any other handlers noted at the same locations) and update all call sites (e.g.,
Task { await createDemoSecret() } to Task { await didTapCreateDemoSecret() })
plus any references in UI button labels, method declarations, and tests; ensure
async/throws signatures remain unchanged and run a build to fix any broken
imports/usages.
- Around line 3-4: The import statements are out of order: move the system
framework import SwiftUI to be first, then add a blank line and place the module
import DVDomain after it so imports follow the guideline (system frameworks
first, blank line, third-party/local modules) — update the import block
containing SwiftUI and DVDomain accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e67782fc-98df-4222-80ea-deddaffed4e3
📒 Files selected for processing (61)
Projects/DVData/Project.swiftProjects/DVData/Sources/RepositoryImpl/Secret/InMemorySecretQueryFilter.swiftProjects/DVData/Sources/RepositoryImpl/Secret/SecretFetchDescriptorBuilder.swiftProjects/DVData/Sources/RepositoryImpl/Secret/SecretRepositoryImpl.swiftProjects/DVData/Sources/ServiceImpl/Authentication/LocalUserAuthenticationServiceImpl.swiftProjects/DVData/Sources/ServiceImpl/Security/JSONCoder/SecretMetadataJSONCoder.swiftProjects/DVData/Sources/ServiceImpl/Security/JSONCoder/SecretPayloadJSONCoder.swiftProjects/DVData/Sources/ServiceImpl/Security/Keychain/KeychainKeyStore.swiftProjects/DVData/Sources/ServiceImpl/Security/SecretCryptoServiceImpl.swiftProjects/DVData/Sources/Storage/Local/LocalStorage.swiftProjects/DVData/Sources/Storage/Local/Models/Secret.swiftProjects/DVData/Sources/Storage/Local/Models/SecretMetadata.swiftProjects/DVData/Sources/Storage/Local/Models/SecretPayload.swiftProjects/DVDesign/Project.swiftProjects/DVDomain/Project.swiftProjects/DVDomain/Sources/Entity/Secret.swiftProjects/DVDomain/Sources/Entity/SecretMetadata.swiftProjects/DVDomain/Sources/Entity/SecretPayload.swiftProjects/DVDomain/Sources/Repository/Error/SecretRepositoryError.swiftProjects/DVDomain/Sources/Repository/Interface/SecretRepository.swiftProjects/DVDomain/Sources/Repository/SecretPatch.swiftProjects/DVDomain/Sources/Repository/SecretQuery.swiftProjects/DVDomain/Sources/SecretContent/Metadata/APIKeyMetadata.swiftProjects/DVDomain/Sources/SecretContent/Metadata/DatabaseMetadata.swiftProjects/DVDomain/Sources/SecretContent/Metadata/Interface/SecretMetadataContent.swiftProjects/DVDomain/Sources/SecretContent/Metadata/LicenseKeyMetadata.swiftProjects/DVDomain/Sources/SecretContent/Metadata/OAuthClientMetadata.swiftProjects/DVDomain/Sources/SecretContent/Metadata/SSHKeyMetadata.swiftProjects/DVDomain/Sources/SecretContent/Metadata/SSLCertMetadata.swiftProjects/DVDomain/Sources/SecretContent/Metadata/ServiceAccountMetadata.swiftProjects/DVDomain/Sources/SecretContent/Payload/APIKeyPayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/CustomPayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/DatabasePayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/EnvSetPayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/Interface/SecretPayloadData.swiftProjects/DVDomain/Sources/SecretContent/Payload/LicenseKeyPayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/OAuthClientPayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/SSHKeyPayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/SSLCertPayload.swiftProjects/DVDomain/Sources/SecretContent/Payload/ServiceAccountPayload.swiftProjects/DVDomain/Sources/Service/Error/SecretCryptoError.swiftProjects/DVDomain/Sources/Service/Error/UserAuthenticationError.swiftProjects/DVDomain/Sources/Service/Interface/SecretCryptoService.swiftProjects/DVDomain/Sources/Service/Interface/UserAuthenticationService.swiftProjects/DVDomain/Sources/UseCase/Draft/SecretDraft.swiftProjects/DVDomain/Sources/UseCase/Error/SecretUseCaseError.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/DeleteSecretUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/FetchSecretUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/SecretUseCaseHelper.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/CreateSecretUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/DeleteSecretUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/FetchSecretUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/PatchSecretUseCase.swiftProjects/DVDomain/Tests/.gitkeepProjects/DVDomain/Tests/ExampleTest.swiftProjects/DVPresentation/Project.swiftProjects/DVPresentation/Sources/SecretUseCaseDemoView.swiftProjects/Devault/Project.swiftProjects/Devault/Sources/ContentView.swift
💤 Files with no reviewable changes (1)
- Projects/DVDomain/Tests/ExampleTest.swift
Comment on lines
+102
to
+104
| let status = bytes.withUnsafeMutableBytes { | ||
| SecRandomCopyBytes(kSecRandomDefault, count, $0.baseAddress!) | ||
| } |
There was a problem hiding this comment.
강제 언래핑 제거 및 baseAddress nil 처리 필요
SecRandomCopyBytes(..., $0.baseAddress!)의 강제 언래핑은 baseAddress가 nil인 경우 크래시 리스크가 있어 guard let로 안전 처리하세요(예: nil이면 errSecParam 반환).
개선 예시
- let status = bytes.withUnsafeMutableBytes {
- SecRandomCopyBytes(kSecRandomDefault, count, $0.baseAddress!)
- }
+ let status = bytes.withUnsafeMutableBytes { buffer -> OSStatus in
+ guard let baseAddress = buffer.baseAddress else {
+ return errSecParam
+ }
+ return SecRandomCopyBytes(kSecRandomDefault, count, baseAddress)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let status = bytes.withUnsafeMutableBytes { | |
| SecRandomCopyBytes(kSecRandomDefault, count, $0.baseAddress!) | |
| } | |
| let status = bytes.withUnsafeMutableBytes { buffer -> OSStatus in | |
| guard let baseAddress = buffer.baseAddress else { | |
| return errSecParam | |
| } | |
| return SecRandomCopyBytes(kSecRandomDefault, count, baseAddress) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Projects/DVData/Sources/ServiceImpl/Security/Keychain/KeychainKeyStore.swift`
around lines 102 - 104, Remove the force-unwrap of baseAddress in the
SecRandomCopyBytes call inside bytes.withUnsafeMutableBytes: replace the direct
$0.baseAddress! usage with a safe guard (e.g., inside the closure do guard let
base = $0.baseAddress else { return errSecParam } and pass base to
SecRandomCopyBytes) so that bytes.withUnsafeMutableBytes { ... } returns
errSecParam when baseAddress is nil; update the surrounding code in
KeychainKeyStore.swift (the bytes.withUnsafeMutableBytes / SecRandomCopyBytes
invocation) to handle the returned status accordingly.
Comment on lines
+197
to
+202
| func revealSelectedPayload() async { | ||
| guard let id = selectedSecret?.id else { return } | ||
|
|
||
| await run("Revealing payload...") { | ||
| revealedPayload = try await fetchSecretUseCase.revealPayload(id: id, as: APIKeyPayload.self) | ||
| statusMessage = "Payload revealed" |
There was a problem hiding this comment.
타입 고정 Reveal로 인해 선택한 secretType과 불일치 시 항상 실패합니다.
Line 201에서 APIKeyPayload.self를 고정 사용하고 있어, API Key가 아닌 레코드를 선택하면 기본적으로 실패 상태가 됩니다. 타입 가드 후 사용자 메시지를 분기하는 편이 안전합니다.
🔧 제안 패치
func revealSelectedPayload() async {
- guard let id = selectedSecret?.id else { return }
+ guard let selectedSecret else { return }
+ guard selectedSecret.secretType == "apiKey" else {
+ statusMessage = "Unsupported payload type for demo reveal"
+ revealedPayload = nil
+ return
+ }
+ let id = selectedSecret.id
await run("Revealing payload...") {
revealedPayload = try await fetchSecretUseCase.revealPayload(id: id, as: APIKeyPayload.self)
statusMessage = "Payload revealed"
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Projects/DVPresentation/Sources/SecretUseCaseDemoView.swift` around lines 197
- 202, In revealSelectedPayload(), the code always calls
fetchSecretUseCase.revealPayload(id:as:) with APIKeyPayload.self which causes
failures when the selectedSecret is not an API key; change the logic to inspect
selectedSecret (e.g., selectedSecret?.type or secretType) and branch by type
(switch or if) to call revealPayload with the matching payload type (or handle
unsupported types), then assign revealedPayload and statusMessage accordingly so
non-APIKey secrets don’t try to decode as APIKeyPayload.self.
doyeonk429
reviewed
May 24, 2026
doyeonk429
requested changes
May 24, 2026
doyeonk429
left a comment
Contributor
There was a problem hiding this comment.
✅ 좋은 점
- 레이어 책임 분리가 깔끔함. Domain은 외부 의존 없이 Entity/UseCase/Service 인터페이스만 정의, Data가 SwiftData/Keychain/LocalAuth 구현 담당. Secret Domain
entity와 SwiftDataModel.Secret 분리 + toDomain() 매핑도 일관됨. - @Modelactor 사용으로 SwiftData 동시성 안전성 확보. SecretRepositoryImpl이 actor로 묶여 있어 ModelContext 격리가 자연스럽게 해결됨.
- PatchField enum의 .unchanged / .set(nil) 구분이 우아함. partial update에서 흔히 망가지는 부분을 잘 잡았습니다.
- keyTag를 payload별로 저장하는 설계 → 향후 key rotation 시 옛 데이터 복호화 가능. payload.keyTag로 복호화하고 getSymmetricKey는 새 키 생성 안 함 — 정확한 선택.
- schemaVersion을 payload/metadata에 박아둠 → 향후 content 구조 변경 마이그레이션 대응 여지 확보.
- idGenerator/dateProvider 주입 → UseCase 테스트 친화적 설계.
- kSecAttrAccessibleWhenUnlockedThisDeviceOnly 선택 적절. iCloud Keychain 동기화 차단 + 잠금 상태에서 접근 차단.
- 에러 계층 분리(Repository/Crypto/Auth/UseCase) + SecretUseCaseError.map의 패스스루(as? SecretUseCaseError 먼저 체크) 처리 깔끔.
doyeonk429
force-pushed
the
feature/#12/Secret_ORM
branch
from
May 24, 2026 14:43
9ab75e8 to
9faeab4
Compare
doyeonk429
approved these changes
Jun 30, 2026
# Conflicts: # Projects/DVDomain/Project.swift
doyeonk429
force-pushed
the
feature/#12/Secret_ORM
branch
from
June 30, 2026 06:11
86a627f to
3f90204
Compare
3 tasks
This was referenced Jul 9, 2026
This was referenced Jul 21, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ What’s this PR?
📌 관련 이슈 (Related Issue)
📁새로 생긴 폴더링과 파일
Domain
Data
Presentation
App
🧶 주요 변경 내용 (Summary)
Task 1. Tuist 모듈 의존성 정리
Task 2. Secret Domain Entity 정의
Task 3. Repository Interface 정의
Task 4. Secret Content 모델 정의
Task 5. Secret UseCase 구성
Task 6. SwiftData RepositoryImpl 구현
Task 7. 암호화/Keychain/JSON 처리 구현
Task 8. 사용자 인증 책임 분리
Task 9. 임시 Demo View 추가
📸 스크린샷 (Optional)
2026-05-23.11.38.10.mov
🧪 테스트 / 검증 내역
💬 기타 공유 사항
Entity 네이밍 고민 -> SecretEntity와 같은 접미사를 붙일 수도 있음.
UseCase 단위 고민 -> 지금 구현한 방식은 “사용자 의도 하나의 작업 단위”로 보는 관점으로.
앞으로 localAuth를 쓰는 곳은 많지만 정해져 있을 테니, authenticate의 reason 파라미터를 String이 아닌 AuthenticationPurpose enum으로 정의해서 받아야 할 것
Metadata, payload의 각 content들은 notion DB설계 페이지에서 미리 정의하고 구현한 내용입니다.
Service 관련
🙇🏻♀️ 리뷰 가이드 (선택)
Summary by CodeRabbit