Skip to content

Share 10-min Touch ID context across org operations #390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JasperNoBoxDev merged 1 commit into from
Apr 10, 2026
+44 −20
Merged

Share 10-min Touch ID context across org operations #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions NoxKey/SocketServer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,35 @@ class SocketServer {
private let acceptQueue = DispatchQueue(label: "noxkey.accept", qos: .userInitiated)
private let clientQueue = DispatchQueue(label: "noxkey.clients", qos: .userInitiated, attributes: .concurrent)

// MARK: - Shared org management auth context (10-min reuse window)
private var orgContext: LAContext?
private var orgContextExpiry: Date = .distantPast
private let orgContextLock = NSLock()

private func orgAuthContext(reason: String) -> LAContext? {
orgContextLock.lock()
defer { orgContextLock.unlock() }
if let ctx = orgContext, Date() < orgContextExpiry {
return ctx
Comment on lines +27 to +35
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Scope the cached LAContext to the caller, not the whole server.

orgContext is shared across every socket client. Once one process authenticates, any other same-UID caller can reuse that context for the next 10 minutes and rename org secrets without its own prompt. That’s a broader trust boundary than the rest of this file, which keys access by resolvedPid and resolvedStartTime. Cache this per caller identity instead of process-wide.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@NoxKey/SocketServer.swift` around lines 27 - 35, orgContext is currently a
global cached LAContext shared across all socket clients which lets any same-UID
caller reuse another caller's auth for 10 minutes; change the cache to be
per-caller identity (keyed by the same identity used elsewhere: resolvedPid and
resolvedStartTime or a single combined CallerID) instead of a single orgContext.
Replace the single-orgContext/orgContextExpiry with a dictionary mapping
CallerID -> (LAContext, expiry) protected by orgContextLock, update
orgAuthContext(reason:) to look up and return the caller-specific LAContext (and
to store a new context under that caller key when created), and update any
invalidation logic to remove only that caller’s entry rather than clearing a
global context.

}
let ctx = LAContext()
ctx.touchIDAuthenticationAllowableReuseDuration = 600 // 10 minutes
let policy = KeychainManager.authPolicy
var authError: NSError?
guard ctx.canEvaluatePolicy(policy, error: &authError) else { return nil }
Comment on lines +40 to +41
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It would be beneficial to log the authError if canEvaluatePolicy fails. This information is crucial for diagnosing why biometric authentication might be unavailable (e.g., biometry locked out, not enrolled, or hardware issues), which is more helpful than a generic failure message.

var ok = false
let sem = DispatchSemaphore(value: 0)
ctx.evaluatePolicy(policy, localizedReason: reason) { success, _ in
ok = success
sem.signal()
}
sem.wait()
guard ok else { return nil }
Comment on lines +42 to +49
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add a timeout to the LocalAuthentication wait path.

This helper blocks on sem.wait() indefinitely while still holding orgContextLock. If evaluatePolicy never calls back, one hung auth attempt stalls all later org renames. Match authenticateForMutation here and fail closed after a bounded timeout.

Suggested fix 
         var ok = false
         let sem = DispatchSemaphore(value: 0)
         ctx.evaluatePolicy(policy, localizedReason: reason) { success, _ in
             ok = success
             sem.signal()
         }
-        sem.wait()
-        guard ok else { return nil }
+        guard sem.wait(timeout: .now() + 60) == .success, ok else { return nil }
         orgContext = ctx
         orgContextExpiry = Date().addingTimeInterval(600)
         return ctx
📝 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
var ok = false
let sem = DispatchSemaphore(value: 0)
ctx.evaluatePolicy(policy, localizedReason: reason) { success, _ in
ok = success
sem.signal()
}
sem.wait()
guard ok else { return nil }
var ok = false
let sem = DispatchSemaphore(value: 0)
ctx.evaluatePolicy(policy, localizedReason: reason) { success, _ in
ok = success
sem.signal()
}
guard sem.wait(timeout: .now() + 60) == .success, ok else { return nil }
orgContext = ctx
orgContextExpiry = Date().addingTimeInterval(600)
return ctx
🧰 Tools
🪛 ast-grep (0.42.1)

[info] 43-43: The application was observed to leverage biometrics via Local
Authentication, which returns a simple boolean result for authentication.
This design is subject to bypass with runtime tampering tools such as
Frida, Substrate, and others. Although this is limited to rooted
(jailbroken) devices, consider implementing biometric authentication the
reliable way - via Keychain Services.
Context: ctx.evaluatePolicy
Note: [CWE-305] Authentication Bypass by Primary Weakness [REFERENCES]
- https://mobile-security.gitbook.io/mobile-security-testing-guide/ios-testing-guide/0x06f-testing-local-authentication
- https://shirazkhan030.medium.com/biometric-authentication-in-ios-6c53c54f17df

(insecure-biometrics-swift)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@NoxKey/SocketServer.swift` around lines 42 - 49, The current evaluatePolicy
callback blocks indefinitely on sem.wait(), risking a permanent hang while
holding orgContextLock; change the synchronous wait to a bounded wait using
DispatchSemaphore.wait(timeout:) with the same timeout used by
authenticateForMutation and treat a timeout as failure (return nil).
Specifically, replace the unconditional sem.wait() with a timed wait
(DispatchTime.now() + the authenticateForMutation timeout constant/value) and
check the wait result before the existing guard ok else { return nil } so that
if the semaphore times out you fail closed.

orgContext = ctx
orgContextExpiry = Date().addingTimeInterval(600)
return ctx
}
Comment on lines +31 to +53
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The orgAuthContext method uses a synchronous wait (sem.wait()) while holding an NSLock. While this correctly prevents multiple simultaneous Touch ID prompts, it blocks the calling thread (likely from clientQueue) for the entire duration of the user interaction. In a server environment, this can lead to thread starvation if multiple requests are pending. Consider using an asynchronous approach or ensuring that the thread pool is sufficiently sized to handle these long-running blocking operations.


// MARK: - Known callers tracking (first-seen warning)
private static var knownCallersPath: String {
guard let home = getpwuid(getuid())?.pointee.pw_dir.flatMap({ String(cString: $0) }) else {
Expand Down Expand Up @@ -1539,16 +1568,11 @@ class SocketServer {
return
}

// Use an authenticated context for reading secret values during rename
let context = LAContext()
let policy = KeychainManager.authPolicy
var authError: NSError?
guard context.canEvaluatePolicy(policy, error: &authError) else {
sendResponse(clientSocket, ok: false, error: "Authentication unavailable for secret migration")
// Use shared org auth context (10-min reuse window — single Touch ID for batch org operations)
guard let context = orgAuthContext(reason: "Rename organization \"\(fromOrg)\" → \"\(toOrg)\" — \(caller.label)") else {
sendResponse(clientSocket, ok: false, error: "Authentication failed for secret migration")
return
Comment on lines +1571 to 1574
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This still won’t deliver the advertised org auth behavior.

orgRename now has a cached context, but handleOrg still runs authenticateForMutation for every action except list before this code is reached. So rename still incurs the outer prompt, and add / remove still require Touch ID. To match the new behavior, remove the outer auth gate for add and remove, and let rename rely on orgAuthContext alone.

Suggested fix outside this hunk 
     private func handleOrg(_ clientSocket: Int32, _ request: [String: Any], _ caller: CallerInfo) {
         let action = request["action"] as? String ?? "list"

-        // "list" needs no auth — everything else gets one Touch ID prompt
-        if action != "list" {
+        // Only actions that mutate Keychain-backed secrets need auth here.
+        // `rename` performs its own cached auth via `orgAuthContext`.
+        if action == "some_future_mutation_requiring_global_auth" {
             guard authenticateForMutation(reason: "Manage organizations — \(caller.label)") else {
                 sendResponse(clientSocket, ok: false, error: "Authentication canceled")
                 return
             }
         }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@NoxKey/SocketServer.swift` around lines 1571 - 1574, The auth flow is still
prompting too early: update handleOrg to stop calling authenticateForMutation
for the "add" and "remove" actions and to avoid the outer
authenticateForMutation before invoking orgRename so that orgRename can solely
rely on orgAuthContext; specifically, remove or short-circuit the
authenticateForMutation invocation in handleOrg for cases "add" and "remove" and
ensure orgRename continues to call orgAuthContext(reason:) (and only falls back
to sendResponse on nil), so that add/remove use the inner orgAuthContext
behavior and rename uses the cached orgAuthContext without an extra outer
prompt.

}
// Reuse the already-approved biometric — no second prompt
context.touchIDAuthenticationAllowableReuseDuration = 30

let secrets = keychain.list(context: context)
let prefix = fromOrg + "/"
Expand Down
12 changes: 6 additions & 6 deletions NoxKey/noxkey-mcp.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27435,9 +27435,9 @@ var server = new McpServer(
"- noxkey_delete \u2014 delete a secret (Touch ID required)",
"",
"- noxkey_org_list \u2014 list all organizations with secret counts",
"- noxkey_org_add \u2014 add an organization (Touch ID required)",
"- noxkey_org_remove \u2014 remove an organization (Touch ID required, secrets are NOT deleted)",
"- noxkey_org_rename \u2014 rename an organization and move all its secrets (Touch ID required)",
"- noxkey_org_add \u2014 add an organization (no Touch ID)",
"- noxkey_org_remove \u2014 remove an organization (no Touch ID, secrets are NOT deleted)",
"- noxkey_org_rename \u2014 rename an organization and move all its secrets (Touch ID once, reused 10 min)",
"",
"Advanced (use only when needed): noxkey_batch_get, noxkey_authorize, noxkey_meta, noxkey_convert, noxkey_session, noxkey_tokens, noxkey_revoke",
"",
Expand Down Expand Up @@ -27741,7 +27741,7 @@ ${lines.join("\n")}`);
);
server.tool(
"noxkey_org_add",
"Add an organization. Requires Touch ID. Organizations group secrets under a shared prefix (e.g. 'mycompany/project/KEY').",
"Add an organization. No Touch ID needed. Organizations group secrets under a shared prefix (e.g. 'mycompany/project/KEY').",
{ org: z2.string().describe("Organization name (e.g. 'mycompany')") },
async ({ org }) => {
const trimmed = org.trim().toLowerCase();
Expand All @@ -27755,7 +27755,7 @@ server.tool(
);
server.tool(
"noxkey_org_remove",
"Remove an organization. Requires Touch ID. Secrets using this org prefix are NOT deleted.",
"Remove an organization. No Touch ID needed. Secrets using this org prefix are NOT deleted.",
{ org: z2.string().describe("Organization name to remove (e.g. 'mycompany')") },
async ({ org }) => {
const trimmed = org.trim().toLowerCase();
Expand All @@ -27767,7 +27767,7 @@ server.tool(
);
server.tool(
"noxkey_org_rename",
"Rename an organization. Requires Touch ID. All secrets under the old org prefix are moved to the new one.",
"Rename an organization. Requires Touch ID (once \u2014 reused for 10 minutes). All secrets under the old org prefix are moved to the new one.",
{
from: z2.string().describe("Current organization name"),
to: z2.string().describe("New organization name")
Expand Down
12 changes: 6 additions & 6 deletions mcp-server/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ const server = new McpServer(
"- noxkey_delete — delete a secret (Touch ID required)",
"",
"- noxkey_org_list — list all organizations with secret counts",
"- noxkey_org_add — add an organization (Touch ID required)",
"- noxkey_org_remove — remove an organization (Touch ID required, secrets are NOT deleted)",
"- noxkey_org_rename — rename an organization and move all its secrets (Touch ID required)",
"- noxkey_org_add — add an organization (no Touch ID)",
"- noxkey_org_remove — remove an organization (no Touch ID, secrets are NOT deleted)",
"- noxkey_org_rename — rename an organization and move all its secrets (Touch ID once, reused 10 min)",
"",
"Advanced (use only when needed): noxkey_batch_get, noxkey_authorize, noxkey_meta, noxkey_convert, noxkey_session, noxkey_tokens, noxkey_revoke",
"",
Expand Down Expand Up @@ -557,7 +557,7 @@ server.tool(

server.tool(
"noxkey_org_add",
"Add an organization. Requires Touch ID. Organizations group secrets under a shared prefix (e.g. 'mycompany/project/KEY').",
"Add an organization. No Touch ID needed. Organizations group secrets under a shared prefix (e.g. 'mycompany/project/KEY').",
{ org: z.string().describe("Organization name (e.g. 'mycompany')") },
async ({ org }) => {
const trimmed = org.trim().toLowerCase();
Expand All @@ -572,7 +572,7 @@ server.tool(

server.tool(
"noxkey_org_remove",
"Remove an organization. Requires Touch ID. Secrets using this org prefix are NOT deleted.",
"Remove an organization. No Touch ID needed. Secrets using this org prefix are NOT deleted.",
{ org: z.string().describe("Organization name to remove (e.g. 'mycompany')") },
async ({ org }) => {
const trimmed = org.trim().toLowerCase();
Expand All @@ -585,7 +585,7 @@ server.tool(

server.tool(
"noxkey_org_rename",
"Rename an organization. Requires Touch ID. All secrets under the old org prefix are moved to the new one.",
"Rename an organization. Requires Touch ID (once — reused for 10 minutes). All secrets under the old org prefix are moved to the new one.",
{
from: z.string().describe("Current organization name"),
to: z.string().describe("New organization name"),
Expand Down