Skip to content

Commit 8f9cfeb

Browse files
jscarlejscarle
authored
Merge pull request #98 from jscarle/feature/remove-share-rawresponse
Improved item sharing and platform-specific CLI handling
2 parents 9e21b38 + 7bd9cb0 commit 8f9cfeb

17 files changed

Lines changed: 303 additions & 1203 deletions

‎AGENTS.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ This repository contains `OnePassword.NET`, a .NET wrapper for the 1Password CLI
88
- If the current branch is not already a feature branch named in the format `feature/<appropriate-short-name>`, create one before committing.
99
- Choose a short, specific branch suffix that describes the work. Keep it lowercase and hyphenated.
1010
- If already on an appropriate `feature/...` branch, do not create an additional branch unless the user asks for one.
11+
- Do not leave your own repository changes uncommitted; commit them before ending the work unless the user explicitly asks you not to commit.
12+
- When creating a commit, include all current unstaged changes in that repository in the commit unless the user explicitly asks to exclude something.
1113
- Commit messages must be a single-line short sentence in past tense that summarizes the commit.
1214
- Commit messages must be written as a proper sentence and must end with a period.
1315
- Do not use multiline commit messages, bullet lists, prefixes, or issue numbers in the commit message unless the user explicitly asks for them.
@@ -24,3 +26,8 @@ This repository contains `OnePassword.NET`, a .NET wrapper for the 1Password CLI
2426

2527
- Do not read, search, or summarize generated documentation/site assets unless the user explicitly asks for them.
2628
- In particular, avoid generated docfx output and bundled vendor assets such as minified JavaScript, CSS, or copied third-party files; prefer the markdown and source files under `docfx/` instead.
29+
30+
## API Abstraction
31+
32+
- Never expose or leak raw 1Password CLI responses through the public API unless the user explicitly asks for that exact behavior.
33+
- Keep the wrapper abstraction stable and consumer-focused: parse CLI output into library models and shield consumers from CLI output-shape changes whenever practical.

‎NEXT_RELEASE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## Breaking changes
44

5-
- `ShareItem(...)` now returns `ItemShareResult` instead of `void`.
5+
- `ShareItem(...)` now returns `ItemShare` instead of `void`.
66

77
## Highlights
88

‎OnePassword.NET.Tests/Common/TestsBase.cs‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,8 @@ public class TestsBase
2727
private static readonly SemaphoreSlim SemaphoreSlim = new(1, 1);
2828
private static readonly CancellationTokenSource TestCancellationTokenSource = new();
2929
private static readonly CancellationTokenSource TearDownCancellationTokenSource = new();
30-
private static readonly bool IsLinux = RuntimeInformation.IsOSPlatform(OSPlatform.Linux);
31-
private static readonly Uri DownloadSource = IsLinux ?
32-
new Uri("https://cache.agilebits.com/dist/1P/op2/pkg/v2.26.0/op_linux_amd64_v2.26.0.zip") :
33-
new Uri("https://cache.agilebits.com/dist/1P/op2/pkg/v2.26.0/op_windows_amd64_v2.26.0.zip");
34-
private static readonly string ExecutableName = IsLinux ? "op" : "op.exe";
30+
private static readonly Uri DownloadSource = GetDownloadSource();
31+
private static readonly string ExecutableName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "op.exe" : "op";
3532
private static bool _initialSetupDone;
3633

3734
private protected static readonly CancellationTokenSource SetUpCancellationTokenSource = new();
@@ -146,6 +143,18 @@ private static string GetEnv(string name, string value)
146143
return value;
147144
}
148145

146+
private static Uri GetDownloadSource()
147+
{
148+
const string version = "v2.32.1";
149+
150+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
151+
return new Uri($"https://cache.agilebits.com/dist/1P/op2/pkg/{version}/op_windows_amd64_{version}.zip");
152+
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
153+
return new Uri($"https://cache.agilebits.com/dist/1P/op2/pkg/{version}/op_darwin_amd64_{version}.zip");
154+
155+
return new Uri($"https://cache.agilebits.com/dist/1P/op2/pkg/{version}/op_linux_amd64_{version}.zip");
156+
}
157+
149158
private protected static void MarkManagementUnsupported()
150159
{
151160
GroupManagementSupported = false;

‎OnePassword.NET.Tests/OnePasswordManagerCommandTests.cs‎

Lines changed: 171 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
using System.Globalization;
2+
using System.IO.Compression;
3+
using System.Reflection;
24
using System.Runtime.InteropServices;
35
using OnePassword.Common;
46
using OnePassword.Documents;
@@ -22,6 +24,15 @@ public void VersionIsTrimmed()
2224
Assert.That(manager.Version, Is.EqualTo("2.32.1"));
2325
}
2426

27+
[Test]
28+
public void DefaultExecutableNameMatchesCurrentPlatform()
29+
{
30+
var method = typeof(OnePasswordManagerOptions).GetMethod("GetDefaultExecutableName", BindingFlags.NonPublic | BindingFlags.Static);
31+
32+
Assert.That(method, Is.Not.Null);
33+
Assert.That(method!.Invoke(null, null), Is.EqualTo(RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "op.exe" : "op"));
34+
}
35+
2536
[Test]
2637
public void ArchiveDocumentObjectOverloadUsesArchiveCommand()
2738
{
@@ -66,6 +77,109 @@ public void ArchiveItemStringOverloadUsesArchiveCommand()
6677
Assert.That(fakeCli.LastArguments, Does.StartWith("item delete item-id --vault vault-id --archive"));
6778
}
6879

80+
[Test]
81+
public void MoveItemStringOverloadUsesResolvedVaultIds()
82+
{
83+
using var fakeCli = new FakeCli();
84+
var manager = fakeCli.CreateManager();
85+
86+
manager.MoveItem("item-id", "current-vault-id", "destination-vault-id");
87+
88+
Assert.Multiple(() =>
89+
{
90+
Assert.That(fakeCli.LastArguments, Does.StartWith("item move item-id --current-vault current-vault-id --destination-vault destination-vault-id"));
91+
Assert.That(fakeCli.LastArguments, Does.Not.Contain("{currentVaultId}"));
92+
Assert.That(fakeCli.LastArguments, Does.Not.Contain("{destinationVaultId}"));
93+
});
94+
}
95+
96+
[Test]
97+
public void SearchForDocumentCreatesMissingOutputDirectory()
98+
{
99+
using var fakeCli = new FakeCli();
100+
var manager = fakeCli.CreateManager();
101+
var outputDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName(), Path.GetRandomFileName());
102+
var outputFilePath = Path.Combine(outputDirectory, "document.txt");
103+
104+
try
105+
{
106+
manager.SearchForDocument("document-id", outputFilePath, "vault-id");
107+
108+
Assert.Multiple(() =>
109+
{
110+
Assert.That(Directory.Exists(outputDirectory), Is.True);
111+
Assert.That(fakeCli.LastArguments, Does.StartWith("document get document-id --out-file "));
112+
Assert.That(fakeCli.LastArguments, Does.Contain(outputFilePath));
113+
Assert.That(fakeCli.LastArguments, Does.Contain(" --force --vault vault-id"));
114+
});
115+
}
116+
finally
117+
{
118+
var rootDirectory = Directory.GetParent(outputDirectory)?.FullName;
119+
if (rootDirectory is not null && Directory.Exists(rootDirectory))
120+
Directory.Delete(rootDirectory, true);
121+
}
122+
}
123+
124+
[Test]
125+
public void GetSecretUsesTrimmedReference()
126+
{
127+
using var fakeCli = new FakeCli();
128+
var manager = fakeCli.CreateManager();
129+
130+
manager.GetSecret(" op://vault/item/field ");
131+
132+
Assert.That(fakeCli.LastArguments, Does.StartWith("read op://vault/item/field --no-newline"));
133+
}
134+
135+
[Test]
136+
public void SaveSecretUsesTrimmedReference()
137+
{
138+
using var fakeCli = new FakeCli();
139+
var manager = fakeCli.CreateManager();
140+
var outputPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
141+
142+
try
143+
{
144+
manager.SaveSecret(" op://vault/item/field ", outputPath);
145+
146+
Assert.That(fakeCli.LastArguments, Does.StartWith("read op://vault/item/field --no-newline --force --out-file "));
147+
Assert.That(fakeCli.LastArguments, Does.Contain(outputPath));
148+
}
149+
finally
150+
{
151+
if (File.Exists(outputPath))
152+
File.Delete(outputPath);
153+
}
154+
}
155+
156+
[Test]
157+
public void RevokeGroupPermissionsUsesVaultGroupCommand()
158+
{
159+
using var fakeCli = new FakeCli();
160+
var manager = fakeCli.CreateManager();
161+
162+
manager.RevokeGroupPermissions("vault-id", "group-id", [VaultPermission.ViewItems]);
163+
164+
Assert.That(fakeCli.LastArguments, Does.StartWith("vault group revoke --vault vault-id --group group-id --permissions "));
165+
Assert.That(fakeCli.LastArguments, Does.Contain("View Items"));
166+
}
167+
168+
[Test]
169+
public void UpdateExtractsCurrentPlatformExecutablePayload()
170+
{
171+
using var fakeCli = new FakeCli(updateVersionOutput: "2.33.0\n");
172+
var manager = fakeCli.CreateManager();
173+
174+
var updated = manager.Update();
175+
176+
Assert.Multiple(() =>
177+
{
178+
Assert.That(updated, Is.True);
179+
Assert.That(manager.Version, Is.EqualTo("2.33.0"));
180+
});
181+
}
182+
69183
[Test]
70184
public void ShareItemWithoutEmailsOmitsEmailsFlag()
71185
{
@@ -77,7 +191,9 @@ public void ShareItemWithoutEmailsOmitsEmailsFlag()
77191
Assert.Multiple(() =>
78192
{
79193
Assert.That(result.Url, Is.EqualTo(new Uri("https://share.example/item")));
80-
Assert.That(result.RawResponse, Is.EqualTo("https://share.example/item"));
194+
Assert.That(result.ExpiresAt, Is.Null);
195+
Assert.That(result.Recipients, Is.Empty);
196+
Assert.That(result.ViewOnce, Is.Null);
81197
Assert.That(fakeCli.LastArguments, Does.StartWith("item share item-id --vault vault-id"));
82198
Assert.That(fakeCli.LastArguments, Does.Not.Contain("--emails"));
83199
});
@@ -93,7 +209,8 @@ public void ShareItemStringSingleEmailOverloadUsesEmailsFlag()
93209

94210
Assert.Multiple(() =>
95211
{
96-
Assert.That(result.RawResponse, Is.EqualTo("{}"));
212+
Assert.That(result.Url, Is.Null);
213+
Assert.That(result.Recipients, Is.Empty);
97214
Assert.That(fakeCli.LastArguments, Does.Contain("--emails recipient@example.com"));
98215
});
99216
}
@@ -190,7 +307,6 @@ public void ShareItemParsesStructuredShareResult()
190307
Assert.That(result.ExpiresAt, Is.EqualTo(DateTimeOffset.Parse("2026-03-15T12:00:00Z", CultureInfo.InvariantCulture)));
191308
Assert.That(result.ViewOnce, Is.True);
192309
Assert.That(result.Recipients, Is.EqualTo(ParsedRecipients));
193-
Assert.That(result.RawResponse, Does.Contain("\"share_link\": \"https://share.example/item\""));
194310
});
195311
}
196312

@@ -199,21 +315,33 @@ private sealed class FakeCli : IDisposable
199315
private readonly string _argumentsPath;
200316
private readonly string _directoryPath;
201317
private readonly string _nextOutputPath;
318+
private readonly string _updateMessagePath;
319+
private readonly string _updatePayloadPath;
320+
private readonly string _updatedVersionOutputPath;
202321
private readonly string _versionOutputPath;
203322

204-
public FakeCli(string versionOutput = "2.32.1\n", string nextOutput = "{}")
323+
public FakeCli(string versionOutput = "2.32.1\n", string nextOutput = "{}", string? updateVersionOutput = null)
205324
{
206325
_directoryPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
207326
_argumentsPath = Path.Combine(_directoryPath, "last-arguments.txt");
208327
_nextOutputPath = Path.Combine(_directoryPath, "next-output.txt");
328+
_updateMessagePath = Path.Combine(_directoryPath, "update-output.txt");
329+
_updatePayloadPath = Path.Combine(_directoryPath, "update-payload.zip");
330+
_updatedVersionOutputPath = Path.Combine(_directoryPath, "updated-version-output.txt");
209331
_versionOutputPath = Path.Combine(_directoryPath, "version-output.txt");
210332

211333
Directory.CreateDirectory(_directoryPath);
212334
File.WriteAllText(_nextOutputPath, nextOutput);
213335
File.WriteAllText(_versionOutputPath, versionOutput);
336+
if (updateVersionOutput is not null)
337+
{
338+
File.WriteAllText(_updateMessagePath, $"Version {updateVersionOutput.Trim()} is now available.");
339+
File.WriteAllText(_updatedVersionOutputPath, updateVersionOutput);
340+
CreateUpdatePayload();
341+
}
214342

215343
var executablePath = Path.Combine(_directoryPath, ExecutableName);
216-
File.WriteAllText(executablePath, GetScript());
344+
File.WriteAllText(executablePath, GetScript("version-output.txt"));
217345
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
218346
{
219347
File.SetUnixFileMode(executablePath,
@@ -243,30 +371,63 @@ public void Dispose()
243371
Directory.Delete(_directoryPath, true);
244372
}
245373

246-
private static string GetScript()
374+
private void CreateUpdatePayload()
375+
{
376+
var updatedExecutablePath = Path.Combine(_directoryPath, PackagedExecutableName);
377+
File.WriteAllText(updatedExecutablePath, GetScript("updated-version-output.txt"));
378+
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
379+
{
380+
File.SetUnixFileMode(updatedExecutablePath,
381+
UnixFileMode.UserRead
382+
| UnixFileMode.UserWrite
383+
| UnixFileMode.UserExecute);
384+
}
385+
386+
using var zipArchive = ZipFile.Open(_updatePayloadPath, ZipArchiveMode.Create);
387+
zipArchive.CreateEntryFromFile(updatedExecutablePath, PackagedExecutableName);
388+
File.Delete(updatedExecutablePath);
389+
}
390+
391+
private static string GetScript(string versionOutputFileName)
247392
{
248393
return RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
249394
? """
250395
@echo off
251396
setlocal
252397
> "%~dp0last-arguments.txt" echo %*
398+
if "%~1"=="update" (
399+
if exist "%~dp0update-payload.zip" copy /y "%~dp0update-payload.zip" "%~3\update-payload.zip" > nul
400+
if exist "%~dp0update-output.txt" type "%~dp0update-output.txt"
401+
exit /b 0
402+
)
253403
if "%~1"=="--version" (
254-
type "%~dp0version-output.txt"
404+
type "%~dp0VERSION_OUTPUT_PLACEHOLDER"
255405
exit /b 0
256406
)
257407
type "%~dp0next-output.txt"
258-
"""
408+
""".Replace("VERSION_OUTPUT_PLACEHOLDER", versionOutputFileName)
259409
: """
260410
#!/bin/sh
261411
script_dir=$(CDPATH= cd -- "$(dirname "$0")" && pwd)
262412
printf '%s' "$*" > "$script_dir/last-arguments.txt"
413+
if [ "$1" = "update" ]; then
414+
if [ -f "$script_dir/update-payload.zip" ]; then
415+
cp "$script_dir/update-payload.zip" "$3/update-payload.zip"
416+
fi
417+
if [ -f "$script_dir/update-output.txt" ]; then
418+
cat "$script_dir/update-output.txt"
419+
fi
420+
exit 0
421+
fi
263422
if [ "$1" = "--version" ]; then
264-
cat "$script_dir/version-output.txt"
423+
cat "$script_dir/VERSION_OUTPUT_PLACEHOLDER"
265424
exit 0
266425
fi
267426
cat "$script_dir/next-output.txt"
268-
""";
427+
""".Replace("VERSION_OUTPUT_PLACEHOLDER", versionOutputFileName);
269428
}
429+
430+
private static string PackagedExecutableName => RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "op.exe" : "op";
270431
}
271432

272433
private sealed class TestDocument(string id) : IDocument

‎OnePassword.NET/IOnePasswordManager.Items.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public ImmutableList<Item> SearchForItems(string? vaultId = null, bool? includeA
153153
/// <param name="viewOnce">Expires the link after a single view.</param>
154154
/// <returns>The created share result.</returns>
155155
/// <exception cref="ArgumentException">Thrown when there is an invalid argument.</exception>
156-
public ItemShareResult ShareItem(IItem item, IVault vault, string emailAddress, TimeSpan? expiresIn = null, bool? viewOnce = null);
156+
public ItemShare ShareItem(IItem item, IVault vault, string emailAddress, TimeSpan? expiresIn = null, bool? viewOnce = null);
157157

158158
/// <summary>Shares an item.</summary>
159159
/// <param name="itemId">The ID of the item to share.</param>
@@ -163,7 +163,7 @@ public ImmutableList<Item> SearchForItems(string? vaultId = null, bool? includeA
163163
/// <param name="viewOnce">Expires the link after a single view.</param>
164164
/// <returns>The created share result.</returns>
165165
/// <exception cref="ArgumentException">Thrown when there is an invalid argument.</exception>
166-
public ItemShareResult ShareItem(string itemId, string vaultId, string emailAddress, TimeSpan? expiresIn = null, bool? viewOnce = null);
166+
public ItemShare ShareItem(string itemId, string vaultId, string emailAddress, TimeSpan? expiresIn = null, bool? viewOnce = null);
167167

168168
/// <summary>Shares an item.</summary>
169169
/// <param name="item">The item to share.</param>
@@ -173,7 +173,7 @@ public ImmutableList<Item> SearchForItems(string? vaultId = null, bool? includeA
173173
/// <param name="viewOnce">Expires the link after a single view.</param>
174174
/// <returns>The created share result.</returns>
175175
/// <exception cref="ArgumentException">Thrown when there is an invalid argument.</exception>
176-
public ItemShareResult ShareItem(IItem item, IVault vault, IReadOnlyCollection<string>? emailAddresses = null, TimeSpan? expiresIn = null, bool? viewOnce = null);
176+
public ItemShare ShareItem(IItem item, IVault vault, IReadOnlyCollection<string>? emailAddresses = null, TimeSpan? expiresIn = null, bool? viewOnce = null);
177177

178178
/// <summary>Shares an item.</summary>
179179
/// <param name="itemId">The ID of the item to share.</param>
@@ -183,5 +183,5 @@ public ImmutableList<Item> SearchForItems(string? vaultId = null, bool? includeA
183183
/// <param name="viewOnce">Expires the link after a single view.</param>
184184
/// <returns>The created share result.</returns>
185185
/// <exception cref="ArgumentException">Thrown when there is an invalid argument.</exception>
186-
public ItemShareResult ShareItem(string itemId, string vaultId, IReadOnlyCollection<string>? emailAddresses = null, TimeSpan? expiresIn = null, bool? viewOnce = null);
186+
public ItemShare ShareItem(string itemId, string vaultId, IReadOnlyCollection<string>? emailAddresses = null, TimeSpan? expiresIn = null, bool? viewOnce = null);
187187
}

‎OnePassword.NET/IOnePasswordManagerOptions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ public interface IOnePasswordManagerOptions
66
/// <summary>The path to the 1Password CLI executable. Defaults to the current working directory.</summary>
77
public string Path { get; set; }
88

9-
/// <summary>The name of the 1Password CLI executable. Defaults to 'op.exe'.</summary>
9+
/// <summary>The name of the 1Password CLI executable. Defaults to 'op.exe' on Windows and 'op' on other platforms.</summary>
1010
public string Executable { get; set; }
1111

1212
/// <summary>When <see langword="true" />, commands sent to the 1Password CLI executable are output to the console. Defaults to <see langword="false" />.</summary>

OnePassword.NET/Items/ItemShareResult.cs renamed to OnePassword.NET/Items/ItemShare.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ namespace OnePassword.Items;
33
/// <summary>
44
/// Represents the result of sharing a 1Password item.
55
/// </summary>
6-
public sealed class ItemShareResult
6+
public sealed class ItemShare
77
{
88
/// <summary>
99
/// The generated share URL.
@@ -24,9 +24,4 @@ public sealed class ItemShareResult
2424
/// Whether the share is view-once, when returned by the CLI.
2525
/// </summary>
2626
public bool? ViewOnce { get; internal set; }
27-
28-
/// <summary>
29-
/// The raw CLI response used to build the result.
30-
/// </summary>
31-
public string RawResponse { get; internal set; } = "";
3227
}

0 commit comments

Comments
 (0)