Skip to content

feat!: winscard: remove scars cache seed and make it global - #755

Open
Pavlo Myroniuk (TheBestTvarynka) wants to merge 4 commits into
masterfrom
fix/macos-scard
Open

Pavlo Myroniuk (TheBestTvarynka) wants to merge 4 commits into
masterfrom
fix/macos-scard

Conversation

@TheBestTvarynka

@TheBestTvarynka Pavlo Myroniuk (TheBestTvarynka) commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Hi,

I fixed the scard logon for system-provided smart cards in this PR. Overall, this PR introduces two major changes:

  1. The scard cache is global now. It means that we have one cache for all contexts and scard handles. The previous approach with cache per context was incorrect.
  2. The scard cache seed was removed to the system-provided smart card (but still present for emulated smart cards). It was a small shock for me when I found out that the scard logon can work well without a seeded cache.

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Cache entries are not isolated by card identifier, initialization can overwrite shared state, and raw cache payloads are logged.

Review effort: Balanced
Findings: 2 High severity · 1 Medium severity

Open (3)
What changed in this PR

Makes smart-card caching process-global while removing cache seeding for system-provided cards.

Changes:

  • Adds a synchronized global cache with freshness handling.
  • Injects the cache into emulated contexts.
  • Simplifies system-card context initialization.
File Description
ffi/​src/​winscard/​system_scard/​context.rs Uses the global cache on non-Windows systems.
ffi/​src/​winscard/​scard_context.rs Injects the global cache and adds write diagnostics.
ffi/​src/​winscard/​piv.rs Updates system-context construction.
ffi/​src/​winscard/​mod.rs Registers the cache module.
ffi/​src/​winscard/​cache.rs Implements global cache storage and tests.
crates/​winscard/​src/​scard_context.rs Accepts a cache implementation and seeds it.
crates/​winscard/​src/​lib.rs Exports the cache abstraction.
crates/​winscard/​src/​cache.rs Defines the public cache trait.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ffi/src/winscard/cache.rs Outdated

#[derive(Default)]
struct ScardCache {
items: BTreeMap<String, CacheItem>,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

First, I decided to support only one smart card per system. But then I found out that I needed to support multiple smart cards. It turned out it wasn't as hard as I thought. Done in ffaf9ba

Now we support multiple system-provided smart cards.
Pay attention: we still support only one smart card per system for emulated smart cards.

Comment thread ffi/src/winscard/scard_context.rs
Comment thread crates/winscard/src/scard_context.rs Outdated
@TheBestTvarynka

Copy link
Copy Markdown
Collaborator Author

Benoît Cortier (@CBenoit), I addressed Copilot's comments and resolved merge conflicts

This branch has not been deployed

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants