diff --git a/README.md b/README.md index 0830c2e..0c2c8d8 100644 --- a/README.md +++ b/README.md @@ -18,12 +18,15 @@ stored 0600 in ~/.config/stele/credentials.json $ stele publish report.html https://stele.example.com/quiet-cedar-otter +expires 2026-08-11 10:31 -$ stele publish report.html --slug q3-report +$ stele publish report.html --slug q3-report --ttl never https://stele.example.com/q3-report +kept until deleted $ stele update q3-report report.html https://stele.example.com/q3-report +kept until deleted $ stele auth status host https://stele.example.com @@ -37,7 +40,9 @@ credential ~/.config/stele/credentials.json Only the URL, the JSON and the skill document go to stdout. Prompts, warnings and errors go to stderr, so `url=$(stele publish page.html)` captures a URL and `stele skill | less` pages a -document. +document. The deadline under the URL is on stderr for that reason, and it is printed at all +because the server's default is *ephemeral* — a page published with no `--ttl` is eventually +unpublished, and a caller handed only a URL would find that out when the link broke. ## Commands @@ -46,8 +51,8 @@ document. | `stele auth login [--host ]` | human, once | Prompts for the token on a TTY, verifies it against the server, writes the credential `0600`. | | `stele auth status` | agent or human | Host, client name, scopes, expiry — never the token. | | `stele auth logout` | human | Forgets the local credential. Does not revoke it. | -| `stele publish [--slug ]` | agent | `POST /pages`. Prints the URL and nothing else. | -| `stele update ` | agent | `PUT /pages/:slug`. Never creates. | +| `stele publish [--slug ] [--ttl ]` | agent | `POST /pages`. Prints the URL on stdout and the page's deadline on stderr. | +| `stele update ` | agent | `PUT /pages/:slug`. Never creates, and never changes the deadline. | | `stele skill` | agent | Proxies `GET /skill`, so the binary keeps zero copies of the contract. | | `stele admin clients create ` | operator | Mints a credential and prints the token **once**. `--scopes`, `--expires-in 90d`. | | `stele admin clients list` | operator | Names, scopes, last use, revocation state. | @@ -58,6 +63,37 @@ operations in `SteleKit` return plain data and print nothing, which is what make rendering choice rather than a second code path — and JSON output is never styled, because it is a machine contract. +## Page lifetimes + +Pages are ephemeral by default. `--ttl` says otherwise: + +| `--ttl` | Means | +| --- | --- | +| omitted | the server's default lifetime, which is a matter of days | +| `30`, `30d` | thirty days | +| `2w` | fourteen days | +| `never` | kept until you delete it | + +Two asymmetries shape the rules here. The first is why the default is ephemeral at all: a page +that outlives its purpose fails quietly and forever, while one that expires too early fails +loudly to somebody who can republish it. The second is why nothing is ever rounded — a lifetime +finer than a day, like `12h`, is refused rather than quietly turned into `1d`, because the +failure mode of guessing is a link that dies on a schedule nobody typed. + +The maximum is deliberately *not* enforced here. That bound belongs to the server's +`PageLifetime`, the same way the slug rules belong to its `Slug`; a copy in this repository +would be a second source of truth that drifts silently the day the server moves it, and the +`400` it earns names the real limit. What this side checks is only what the server cannot: that +you wrote something meaning a number of days, and that turning it into one does not overflow. + +A page's deadline is fixed when it is published. `stele update` has no `--ttl` — replacing a +page's body cannot buy the link another week — and the server answers `?ttl=` on `PUT` with a +`400` rather than a `200` that silently ignored it. Republish if you need a different lifetime. + +The deadline itself is always read off the response, never computed here: the server resolves it +against *its* clock at the moment of the upload, applies its own default when you say nothing, +and on `update` reports the deadline the page already had — a date this side never knew. + ## Exit codes The primary reader of this tool is an agent, and an agent branches on `$?` before it reads diff --git a/Sources/SteleKit/Credentials.swift b/Sources/SteleKit/Credentials.swift index d6e5294..89c618b 100644 --- a/Sources/SteleKit/Credentials.swift +++ b/Sources/SteleKit/Credentials.swift @@ -52,10 +52,15 @@ public struct SteleHost: Sendable, Hashable, Comparable, CustomStringConvertible self.value = canonical } - /// Builds a URL under this host. `path` is expected to start with `/`. + /// Builds a URL under this host. `path` is expected to start with `/` and to be already + /// percent-encoded — `SteleClient.Path` is the only thing that builds one, and it encodes + /// the parts that come from a slug or a credential name itself. Assigning to `path` here + /// instead would be the bug it is guarding against: that setter treats `/` and `.` as the + /// path characters they are, so an encoded segment would have its `%` escaped back into + /// `%25` and a slug of `../admin/clients` would sail through as three real segments. public func url(path: String, query: [URLQueryItem] = []) -> URL? { guard var components = URLComponents(string: value) else { return nil } - components.path = path + components.percentEncodedPath = path components.queryItems = query.isEmpty ? nil : query return components.url } @@ -243,10 +248,25 @@ public struct Credentials: Sendable, Equatable { /// hand-edited file — which is a state worth handling, since the file is documented and /// people edit it. public mutating func set(_ credential: Credential, makeDefault: Bool = true) { + // Which host was already answering bare commands, read *before* the insert. It is not + // simply `defaultHost`: one stored host resolves with no marker at all, and `encode` + // leaves the marker out of a one-host file for that reason — so `defaultHost` is + // routinely nil while a host is plainly the incumbent. Treating that nil as "no default + // yet" is how declining the default used to hand it to the new host, sending the next + // bare `stele publish` to the deployment that asked not to be preferred. + let incumbent = defaultHost ?? (entries.count == 1 ? entries.keys.first : nil) entries[credential.host] = Entry( clientName: credential.clientName, token: credential.token ) - if makeDefault || defaultHost == nil { defaultHost = credential.host } + if makeDefault { + defaultHost = credential.host + } else { + // The incumbent keeps it. With nothing to keep it — a fresh file, or a hand-edited + // one holding several hosts and no marker — the new host takes it only when it is + // the only host stored. Otherwise the file stays ambiguous and `resolve` says so, + // which is the honest answer: nothing here knows which of the others was meant. + defaultHost = incumbent ?? (entries.count == 1 ? credential.host : nil) + } } /// Forgets a host's credential. Returns false if there was nothing stored for it, so diff --git a/Sources/SteleKit/Inputs.swift b/Sources/SteleKit/Inputs.swift index c9503aa..9d28407 100644 --- a/Sources/SteleKit/Inputs.swift +++ b/Sources/SteleKit/Inputs.swift @@ -124,3 +124,135 @@ public enum ExpiryDuration { return TimeInterval(total) } } + +/// How long a published page lives, as the `?ttl=` value `POST /pages` wants. +/// +/// The server measures a page's life in whole days or not at all: `?ttl=30`, or `?ttl=never`. +/// Omitting it is a third thing — the server's own default, seven days — and that default is +/// deliberately not repeated here. A CLI that printed "expires in 7 days" from a constant would +/// keep printing it the day the server changed its mind, and the server tells us the real answer +/// in the response body anyway. +/// +/// Note what this type does *not* do: it has no maximum. The server's `PageLifetime` owns that +/// bound, for the same reason its `Slug` owns the slug rules — a copy here would be a second +/// source of truth that drifts silently the first time the server moves it. An over-long +/// lifetime comes back as a `400` naming the real limit. What is checked here is only what the +/// server cannot check for us: that the caller wrote something that means a number of days at +/// all, and that turning it into one does not overflow on the way. +public enum PageTTL: Sendable, Equatable { + case days(Int) + case never + + /// The query parameter, and the spelling that opts out of expiry. Both are the server's + /// words, and both are exact-match contracts with another repository — the same class of + /// constant as `SteleClient.Path` and `expiresIn`, and the same failure if they drift: a + /// `ttl=forever` would be a `400`, but a parameter named anything but `ttl` would be + /// *ignored*, and a page the caller asked to keep forever would quietly die in a week. + public static let queryParameter = "ttl" + public static let neverKeyword = "never" + + /// Days and weeks, which are the units that survive the trip. + /// + /// Nothing shorter is offered, and that is the whole reason this is not `ExpiryDuration`. + /// The server stores a page's deadline to the day, so `12h` could only be honoured by + /// rounding it to something the caller did not type — and a lifetime silently rounded is + /// the failure both ends of this contract are built to refuse. + static let units: [(suffix: String, days: Int, name: String)] = [ + ("d", 1, "days"), + ("w", 7, "weeks"), + ] + + public enum ParseError: Error, Equatable, CustomStringConvertible { + case malformed(String) + case notPositive(String) + /// A unit this server cannot store a page to: hours, minutes, seconds. Its own case + /// because the caller wrote a perfectly good duration and the answer is not "write a + /// duration" but "days are the resolution you get". + case tooFine(String, unit: String) + /// A number of days too large to be turned into one at all. Not a policy limit — the + /// server's is enforced by the server — but the point past which the arithmetic here + /// stops being arithmetic. + case unreachable(String) + + public var description: String { + switch self { + case .malformed(let raw): + return """ + '\(raw)' is not a page lifetime. Write a whole number of days — 30, or 30d — \ + or '\(PageTTL.neverKeyword)' for a page that is kept until you delete it. + """ + case .notPositive(let raw): + return """ + '\(raw)' is not a lifetime. A page that expires now or in the past would be \ + published dead; use '\(PageTTL.neverKeyword)' for one that is kept. + """ + case .tooFine(let raw, let unit): + return """ + '\(raw)' is \(unit), and a page's lifetime is measured in whole days — \ + honouring it would mean rounding to a lifetime you did not ask for. Write \ + the number of days instead, or '\(PageTTL.neverKeyword)' to keep the page. + """ + case .unreachable(let raw): + return """ + '\(raw)' is too many days to be a lifetime. Use \ + '\(PageTTL.neverKeyword)' for a page that should never expire. + """ + } + } + } + + /// `"30"` and `"30d"` → `.days(30)`; `"2w"` → `.days(14)`; `"never"` → `.never`. + /// + /// A bare number is accepted here where `--expires-in` refuses one, and the difference is + /// not an inconsistency. That flag's underlying unit is *seconds* and its range runs from + /// seconds to weeks, so `90` genuinely has two readings and guessing between them mints a + /// credential that dies in a minute and a half. A page's lifetime has one unit, the server's + /// own document teaches `?ttl=30`, and refusing the spelling an agent just read there would + /// be a second grammar to no purpose. + public static func parse(_ raw: String) throws(ParseError) -> PageTTL { + let text = raw.trimmingCharacters(in: .whitespaces).lowercased() + guard !text.isEmpty else { throw .malformed(raw) } + if text == neverKeyword { return .never } + + // A trailing unit is optional; its absence means days. Anything that is not a digit run + // has to be named as a unit before it can be refused as one, so hours and friends are + // matched here rather than falling through to "that is not a lifetime". + let (digits, multiplier): (Substring, Int) + if let last = text.last, last.isASCII, !last.isNumber { + if let unit = units.first(where: { $0.suffix == String(last) }) { + (digits, multiplier) = (text.dropLast(), unit.days) + } else if let finer = ExpiryDuration.units.first(where: { $0.suffix == String(last) }) { + throw .tooFine(raw, unit: finer.name) + } else { + throw .malformed(raw) + } + } else { + (digits, multiplier) = (text[...], 1) + } + + // ASCII digits specifically, matching `ExpiryDuration`: `isNumber` admits other scripts' + // digits, which `Int` then declines to parse, and the check below reads a failed parse + // as a number too large rather than a number in the wrong script. + guard !digits.isEmpty, digits.allSatisfy({ $0.isASCII && $0.isNumber }) else { + // A leading `-` lands here. It is not a malformed lifetime, it is a negative one, + // and saying so sends the caller to the right correction. + if digits.first == "-", digits.dropFirst().allSatisfy({ $0.isASCII && $0.isNumber }) { + throw .notPositive(raw) + } + throw .malformed(raw) + } + guard let count = Int(digits) else { throw .unreachable(raw) } + guard count > 0 else { throw .notPositive(raw) } + let (days, overflowed) = count.multipliedReportingOverflow(by: multiplier) + guard !overflowed else { throw .unreachable(raw) } + return .days(days) + } + + /// What travels as `?ttl=`. + public var queryValue: String { + switch self { + case .days(let count): return String(count) + case .never: return PageTTL.neverKeyword + } + } +} diff --git a/Sources/SteleKit/Models.swift b/Sources/SteleKit/Models.swift index e966d3c..67507a9 100644 --- a/Sources/SteleKit/Models.swift +++ b/Sources/SteleKit/Models.swift @@ -9,9 +9,45 @@ public struct PageLocation: Codable, Sendable, Equatable { public let slug: String public let url: String - public init(slug: String, url: String) { + /// When the page stops being served, or nil for one that is kept until it is deleted. + /// + /// Reading this rather than computing it from the `--ttl` that was sent is the point: the + /// server resolves the deadline against *its* clock at the moment of the upload, it applies + /// its own default when the caller expressed no opinion, and on `update` it reports the + /// deadline the page already had — a number this side never knew. + /// + /// Nil covers two wire shapes, and they mean the same thing. The server sends an explicit + /// `null` for a permanent page; a deployment older than page expiry omits the key, and on + /// that server nothing expires either. Both are honestly "no deadline". + public let expiresAt: Date? + + private enum CodingKeys: String, CodingKey { + case slug, url + /// The server's spelling. One key on the wire in both directions, so the `--json` this + /// tool prints is the shape it reads. + case expiresAt = "expires" + } + + public init(slug: String, url: String, expiresAt: Date? = nil) { self.slug = slug self.url = url + self.expiresAt = expiresAt + } + + /// Hand-written for the one line in the middle, and for the reason the server hand-writes + /// its own encoder: the synthesised version calls `encodeIfPresent`, which *drops the key* + /// when there is no deadline — so `stele publish --json` on a permanent page would emit a + /// blob with no `expires` in it, and whatever read that blob could not tell "this page is + /// kept" from "this tool has nothing to say about lifetimes". An explicit null says which. + public func encode(to encoder: any Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(slug, forKey: .slug) + try container.encode(url, forKey: .url) + if let expiresAt { + try container.encode(expiresAt, forKey: .expiresAt) + } else { + try container.encodeNil(forKey: .expiresAt) + } } } diff --git a/Sources/SteleKit/SteleClient.swift b/Sources/SteleKit/SteleClient.swift index b41ecd8..ddeab7d 100644 --- a/Sources/SteleKit/SteleClient.swift +++ b/Sources/SteleKit/SteleClient.swift @@ -61,10 +61,38 @@ public struct SteleClient: Sendable { public static let whoami = "/admin/whoami" public static let clients = "/admin/clients" - public static func page(slug: String) -> String { "\(pages)/\(slug)" } + public static func page(slug: String) -> String { "\(pages)/\(segment(slug))" } /// One credential, addressed by name — the handle every admin route uses. `GET` is not /// offered; `DELETE` here is what revokes. - public static func client(named name: String) -> String { "\(clients)/\(name)" } + public static func client(named name: String) -> String { "\(clients)/\(segment(name))" } + + /// Percent-encodes one path segment, so that a name the caller typed stays one segment. + /// + /// Interpolating it raw is route confusion waiting to happen: `/` and `.` are ordinary + /// path characters, and RFC 3986 resolves dot segments before the request goes out, so + /// `stele update ../admin/clients page.html` asks for `PUT /admin/clients` and + /// `stele admin clients revoke ../../pages/x` asks for `DELETE /pages/x`. The credential + /// still only travels to its own host, so nothing leaks — but the resource acted on is + /// not the one named, which is its own kind of bad afternoon. + /// + /// This is encoding, not validation: which slugs are legal remains the server's `Slug` + /// type's business, and an illegal one still comes back as a `400` naming the rule it + /// broke. A slug that needed encoding was never going to be a real slug anyway. + static func segment(_ raw: String) -> String { + let encoded = raw.addingPercentEncoding(withAllowedCharacters: segmentAllowed) ?? "" + // The other half of dot-segment removal, which needs no `/` to bite: `/pages/..` + // resolves to `/` on its own, and `/pages/.` to `/pages`. + guard !encoded.isEmpty, encoded.allSatisfy({ $0 == "." }) else { return encoded } + return String(repeating: "%2E", count: encoded.count) + } + + /// Everything `URLComponents` would leave alone in a path, less `/` — a separator is + /// exactly what a segment must not be able to introduce. + private static let segmentAllowed: CharacterSet = { + var allowed = CharacterSet.urlPathAllowed + allowed.remove("/") + return allowed + }() } /// The type a page body is sent as when the caller expresses no opinion. @@ -83,13 +111,22 @@ public struct SteleClient: Sendable { /// through — and a copy of them in this repository would be a second source of truth that /// drifts silently the first time the server relaxes one. A bad slug comes back as a `400` /// whose message says which rule it broke. + /// + /// `ttl` is nil for "no opinion", which is not the same as `.never`: an absent parameter + /// takes the server's own default lifetime, and only `.never` asks for a page that is kept + /// until it is deleted. The returned `PageLocation` carries the deadline the server settled + /// on either way, which is the only place the answer exists. public func publish( page: Data, contentType: String = SteleClient.defaultContentType, slug: String? = nil, + ttl: PageTTL? = nil, using credential: Credential ) async throws -> PageLocation { - let query = slug.map { [URLQueryItem(name: "slug", value: $0)] } ?? [] + var query = slug.map { [URLQueryItem(name: "slug", value: $0)] } ?? [] + if let ttl { + query.append(URLQueryItem(name: PageTTL.queryParameter, value: ttl.queryValue)) + } return try await send( method: "POST", path: Path.pages, @@ -103,6 +140,10 @@ public struct SteleClient: Sendable { } /// `PUT /pages/:slug`. Replaces a page that already exists; never creates one. + /// + /// Takes no `ttl`, and not by omission — the server answers `?ttl=` on this verb with a + /// `400`. A page's deadline is fixed when it is published, so replacing the body cannot + /// quietly buy the link another week. The response still reports the deadline the page has. public func update( slug: String, page: Data, diff --git a/Sources/stele/PageCommands.swift b/Sources/stele/PageCommands.swift index b7066bc..aa87fe5 100644 --- a/Sources/stele/PageCommands.swift +++ b/Sources/stele/PageCommands.swift @@ -41,6 +41,21 @@ enum PageIO { // Deliberately bare. A prefix, a checkmark or a colour here would all have to be // stripped by whatever consumes it, and the URL is the answer. Terminal.out(location.url) + // The deadline goes to stderr, because stdout is the URL and nothing else. Printed at + // all because the server's default is *ephemeral* — a page published with no --ttl is + // eventually unpublished, and a caller handed only a URL would find that out when the + // link broke. The date comes from the response rather than from a default repeated + // here, so this line stays true on the day the server changes its mind. + Terminal.error(options.style.dim(lifetime(location))) + } + + /// How a page's deadline reads under the URL. + /// + /// Absolute, never "in 7 days", for the reason `Format.moment` already gives: the follow-up + /// question to a relative date is always "so what date is that". + private static func lifetime(_ location: PageLocation) -> String { + guard let expiry = location.expiresAt else { return "kept until deleted" } + return "expires \(Format.moment(expiry))" } } @@ -55,6 +70,10 @@ struct PublishCommand: SteleCommand { --content-type. That decision belonging to the CLI is the point: the `curl` recipe \ this replaces needed an explicit header, and sending the wrong one earned a 415. + Pages are ephemeral unless you say otherwise. With no --ttl the server applies its \ + own default lifetime and the page eventually stops being served; --ttl never keeps \ + it. Whichever it is, the deadline is printed on stderr under the URL. + Exits 5 if --slug is already taken — choose another, or omit it and let the server \ allocate a three-word one. """ @@ -85,13 +104,35 @@ struct PublishCommand: SteleCommand { ) var contentType: String? + @Option( + name: .long, + help: ArgumentHelp( + "How long the page lives: a number of days, or 'never'.", + discussion: """ + 30 and 30d both mean thirty days; 2w means fourteen. Omit it to take the \ + server's default, which is a matter of days — pass 'never' for a link you \ + expect to keep. The server has the last word on the maximum. + """, + valueName: "days" + ), + completion: .list([PageTTL.neverKeyword, "7", "30", "90"]) + ) + var ttl: String? + func execute() async throws { let page = try PageIO.read(file) + // Parsed before the file is sent rather than after: a lifetime this side can already + // tell is unusable should not cost an upload, and the message it earns here names the + // spelling to use instead. + let lifetime = try ttl.map { raw -> PageTTL in + do { return try PageTTL.parse(raw) } catch { throw Failure("\(error)") } + } let credential = try options.credential() let location = try await SteleClient(credential: credential).publish( page: page, contentType: contentType ?? ContentType.inferred(fromPath: file), slug: slug, + ttl: lifetime, using: credential ) try PageIO.report(location, options: options) @@ -106,6 +147,10 @@ struct UpdateCommand: SteleCommand { Never creates a page: an update to a slug that does not exist exits 7 rather than \ quietly publishing at a URL nobody has seen. Publish it with `stele publish --slug \ ` first if that is what you meant. + + There is no --ttl here. A page's lifetime is fixed when it is published, so \ + replacing the body cannot buy the link more time; the deadline it already has is \ + printed under the URL. Republish it if you need a different one. """ ) diff --git a/Tests/SteleKitTests/CredentialsTests.swift b/Tests/SteleKitTests/CredentialsTests.swift index 91449fb..7b7d3d9 100644 --- a/Tests/SteleKitTests/CredentialsTests.swift +++ b/Tests/SteleKitTests/CredentialsTests.swift @@ -241,6 +241,61 @@ struct HostResolutionTests { #expect(throws: CredentialsError.self) { try credentials.resolve() } } + /// The regression this suite exists for on the write side. A one-host file carries no + /// `default` marker — nothing needs one — so the host that has been answering every bare + /// command reloads with `defaultHost == nil`. Reading that nil as "no default yet" is how + /// `--no-default` used to hand the default to the very host that declined it, quietly + /// repointing the next `stele publish` at the deployment just added. + @Test("--no-default leaves the incumbent default alone, marker on disk or not") + func decliningKeepsTheIncumbent() throws { + let home = try FakeHome() + var first = Credentials() + first.set(try credential("https://one.example.com", client: "one")) + try home.store.save(first) + + var reloaded = try home.store.load() + #expect(reloaded.defaultHost == nil) // one host: the file says nothing, and need not + reloaded.set(try credential("https://two.example.com", client: "two"), makeDefault: false) + + let one = try SteleHost("https://one.example.com") + #expect(reloaded.defaultHost == one) + #expect(try reloaded.resolve().host == one) + try home.store.save(reloaded) + #expect(try home.store.load().resolve().clientName == "one") + } + + /// Declining when there is genuinely nothing to decline in favour of. A hand-edited file + /// with several hosts and no marker has no incumbent — `resolve` refuses to guess between + /// them — and adding a third with `--no-default` must not resolve that by promoting the new + /// one. The ambiguity is the user's to settle, and the error names the command that does it. + @Test("--no-default into an ambiguous file leaves it ambiguous") + func decliningWithNoIncumbent() throws { + let json = """ + { + "https://one.example.com": {"client": "one", "token": "stele_pat_a"}, + "https://two.example.com": {"client": "two", "token": "stele_pat_b"} + } + """ + var credentials = try CredentialStore.decode(Data(json.utf8), path: "test") + credentials.set(try credential("https://three.example.com", client: "three"), makeDefault: false) + + #expect(credentials.defaultHost == nil) + #expect(throws: CredentialsError.self) { try credentials.resolve() } + } + + /// The first login is the exception that keeps the rest simple: with nothing stored, the + /// host being added is the only one there is, so it takes the default even when asked not + /// to. Anything else would make `auth login --no-default` on a fresh machine store a + /// credential that no bare command can reach. + @Test("--no-default on a fresh file still leaves a default behind") + func decliningOnAFreshFile() throws { + var credentials = Credentials() + credentials.set(try credential("https://only.example.com", client: "solo"), makeDefault: false) + + #expect(credentials.defaultHost == (try SteleHost("https://only.example.com"))) + #expect(try credentials.resolve().clientName == "solo") + } + @Test("an explicit host wins over the default") func overrideWins() throws { var credentials = Credentials() diff --git a/Tests/SteleKitTests/InputsTests.swift b/Tests/SteleKitTests/InputsTests.swift index 20104b0..1af5f6c 100644 --- a/Tests/SteleKitTests/InputsTests.swift +++ b/Tests/SteleKitTests/InputsTests.swift @@ -125,3 +125,97 @@ struct InputsTests { } } } + +/// The other lifetime, which is a different grammar for a reason: a credential's is seconds and +/// a page's is whole days, and the two flags are close enough together on the command line that +/// where they diverge is worth pinning. +@Suite("page lifetimes") +struct PageTTLTests { + static let dayCases: [(String, Int)] = [ + ("30", 30), ("30d", 30), ("1", 1), ("2w", 14), ("365d", 365), + // Trimmed and lowercased before anything looks at it, like its sibling. + (" 7D ", 7), + ] + + @Test("a lifetime is a whole number of days, with or without a unit", arguments: dayCases) + func days(_ raw: String, _ expected: Int) throws { + #expect(try PageTTL.parse(raw) == .days(expected)) + } + + /// The bare number is the case worth stating outright, because it is exactly what + /// `--expires-in` refuses. A page's lifetime has one unit and the server's own document + /// teaches `?ttl=30`, so the spelling an agent just read there has to work. + @Test("a bare number means days, unlike --expires-in") + func bareNumberIsDays() throws { + #expect(try PageTTL.parse("30") == .days(30)) + #expect(throws: ExpiryDuration.ParseError.malformed("30")) { + try ExpiryDuration.seconds(from: "30") + } + } + + @Test("'never' is the only way to ask for a page that is kept") + func never() throws { + #expect(try PageTTL.parse(PageTTL.neverKeyword) == .never) + #expect(try PageTTL.parse("NEVER") == .never) + // Not a synonym. The server matches one keyword exactly, and a parser that quietly + // accepted a second spelling would send a `400` for a page the caller meant to keep. + #expect(throws: PageTTL.ParseError.malformed("forever")) { try PageTTL.parse("forever") } + } + + /// The heart of why this is not `ExpiryDuration`. `12h` is a perfectly good duration that + /// this server cannot store, and the only ways to honour it are to round — silently giving + /// the caller a lifetime they did not type — or to say so. + @Test( + "a unit finer than a day is refused rather than rounded", + arguments: [("12h", "hours"), ("90m", "minutes"), ("3600s", "seconds")] + ) + func refusesSubDayUnits(_ raw: String, _ unit: String) { + #expect(throws: PageTTL.ParseError.tooFine(raw, unit: unit)) { try PageTTL.parse(raw) } + // And the message says which unit it was, so the correction is arithmetic the caller + // can do rather than a guess at what the tool wanted. + #expect(PageTTL.ParseError.tooFine(raw, unit: unit).description.contains(unit)) + } + + @Test( + "anything that is not a number of days is refused", + arguments: ["", " ", "7.5", "abc", "d", "w", "30x", "3 0", "٣٠", "-", "1e3"] + ) + func refusesMalformed(_ raw: String) { + #expect(throws: PageTTL.ParseError.self) { try PageTTL.parse(raw) } + } + + /// Zero and negative get their own case for the same reason `ExpiryDuration` gives them one: + /// the syntax was right and the correction is a different sentence. + @Test("a lifetime of zero or less is refused as its own case", arguments: ["0", "0d", "-5", "-5d"]) + func refusesNonPositive(_ raw: String) { + #expect(throws: PageTTL.ParseError.notPositive(raw)) { try PageTTL.parse(raw) } + } + + /// This type deliberately has no maximum — the server owns that bound. What it does have is + /// arithmetic, and the arithmetic must not trap: `Int(_:)` on a digit run too long, and the + /// weeks multiplication, are the two places a number can run off the end. + @Test( + "a number too large to become days is reported, not trapped", + arguments: ["999999999999999999999999", "9999999999999999999w", "\(Int.max)w"] + ) + func refusesUnreachable(_ raw: String) { + #expect(throws: PageTTL.ParseError.unreachable(raw)) { try PageTTL.parse(raw) } + } + + /// A lifetime past the *server's* maximum is not rejected here. A copy of that bound would + /// be a second source of truth that drifts the day the server moves it, and the `400` it + /// earns names the real limit — which a local guess could not. + @Test("a lifetime past the server's ceiling is left for the server to refuse") + func doesNotCopyTheServersCeiling() throws { + #expect(try PageTTL.parse("36501") == .days(36_501)) + #expect(try PageTTL.parse("100000d") == .days(100_000)) + } + + @Test("what travels on the wire is the server's own spelling") + func queryValues() { + #expect(PageTTL.days(30).queryValue == "30") + #expect(PageTTL.days(14).queryValue == "14") + #expect(PageTTL.never.queryValue == "never") + #expect(PageTTL.queryParameter == "ttl") + } +} diff --git a/Tests/SteleKitTests/SteleClientTests.swift b/Tests/SteleKitTests/SteleClientTests.swift index cc26543..ae6f8c8 100644 --- a/Tests/SteleKitTests/SteleClientTests.swift +++ b/Tests/SteleKitTests/SteleClientTests.swift @@ -87,6 +87,54 @@ struct SteleClientRequestTests { #expect(try #require(await transport.last).url.query == "slug=my-page") } + /// The silent failure this parameter is heir to. A `ttl` misspelled in the query is not a + /// `400` — the server has no reason to look at a parameter it does not know, so the upload + /// earns a cheerful `201` and the page the caller asked to keep quietly dies on the default + /// schedule. Read off the URL, because nothing downstream would notice. + @Test( + "a lifetime travels as ?ttl=, spelled the server's way", + arguments: [(PageTTL.days(30), "ttl=30"), (PageTTL.days(14), "ttl=14"), (.never, "ttl=never")] + ) + func lifetimeQuery(_ ttl: PageTTL, _ expected: String) async throws { + let transport = FakeTransport(status: 201, body: #"{"slug":"a-b-c","url":"u","expires":null}"#) + let credential = try testCredential() + let client = SteleClient(credential: credential, transport: transport) + + _ = try await client.publish(page: Data("x".utf8), ttl: ttl, using: credential) + + #expect(try #require(await transport.last).url.query == expected) + } + + /// `?ttl=` and `?slug=` are independent, and a page that asks for both must send both — the + /// query is built by appending, and an assignment where an append belonged would drop one. + @Test("a slug and a lifetime both travel") + func slugAndLifetime() async throws { + let transport = FakeTransport(status: 201, body: #"{"slug":"my-page","url":"u","expires":null}"#) + let credential = try testCredential() + let client = SteleClient(credential: credential, transport: transport) + + _ = try await client.publish( + page: Data("x".utf8), slug: "my-page", ttl: .never, using: credential + ) + + let query = try #require(await transport.last).url.query + #expect(query == "slug=my-page&ttl=never") + } + + /// No `--ttl` means no parameter at all, which is a third thing and not a synonym for + /// either value: absence is what asks for the server's default lifetime. `?ttl=` with + /// nothing after it is a `400`, and `?ttl=never` would keep a page nobody asked to keep. + @Test("publishing without a lifetime sends no ttl at all") + func noLifetime() async throws { + let transport = FakeTransport(status: 201, body: #"{"slug":"a-b-c","url":"u","expires":null}"#) + let credential = try testCredential() + let client = SteleClient(credential: credential, transport: transport) + + _ = try await client.publish(page: Data("x".utf8), slug: "a-b-c", using: credential) + + #expect(try #require(await transport.last).url.query == "slug=a-b-c") + } + @Test("update puts to the slug's own path and sends no query") func update() async throws { let transport = FakeTransport(status: 200, body: #"{"slug":"my-page","url":"u"}"#) @@ -100,6 +148,52 @@ struct SteleClientRequestTests { #expect(request.url.absoluteString == "https://stele.example.com/pages/my-page") } + /// A slug is one path segment and must stay one, however it is spelled. Left raw it is not + /// only a `/` away from another route: RFC 3986 removes dot segments before the request is + /// ever sent, so `update ../admin/clients` reaches the server as `PUT /admin/clients` — an + /// admin route addressed by a page command. The token still travels only to its own host, + /// so this is not a leak; it is the request acting on a resource nobody named. + @Test( + "a slug cannot climb out of /pages", + arguments: [ + ("../admin/clients", "https://stele.example.com/pages/..%2Fadmin%2Fclients"), + ("..", "https://stele.example.com/pages/%2E%2E"), + (".", "https://stele.example.com/pages/%2E"), + ("a/b", "https://stele.example.com/pages/a%2Fb"), + ("quiet-cedar-otter", "https://stele.example.com/pages/quiet-cedar-otter"), + ] + ) + func slugStaysOneSegment(_ slug: String, _ expected: String) async throws { + let transport = FakeTransport(status: 200, body: #"{"slug":"x","url":"u"}"#) + let credential = try testCredential() + let client = SteleClient(credential: credential, transport: transport) + + _ = try? await client.update(slug: slug, page: Data("x".utf8), using: credential) + + let request = try #require(await transport.last) + #expect(request.url.absoluteString == expected) + // The property the escapes above are only instances of: whatever was typed, the request + // is still for something under /pages. + #expect(request.url.standardized.path.hasPrefix("/pages/")) + } + + /// The same rule on the admin side, where the reachable routes are the destructive ones. + @Test("a client name cannot climb out of /admin/clients") + func clientNameStaysOneSegment() async throws { + let transport = FakeTransport(status: 200, body: #"{"name":"x","scopes":[]}"#) + let credential = try testCredential() + let client = SteleClient(credential: credential, transport: transport) + + _ = try? await client.revokeClient(name: "../../pages/quiet-cedar-otter", using: credential) + + let request = try #require(await transport.last) + #expect( + request.url.absoluteString + == "https://stele.example.com/admin/clients/..%2F..%2Fpages%2Fquiet-cedar-otter" + ) + #expect(request.url.standardized.path.hasPrefix("/admin/clients/")) + } + /// The skill is a read, and reads on this server are unauthenticated — an agent /// bootstrapping from it does not have a credential yet. @Test("fetching the skill sends no credential") @@ -333,6 +427,71 @@ struct SteleClientRequestTests { } } +/// The other half of the lifetime contract: what comes back. The deadline is the server's to +/// compute — against its clock, with its default — so reading it off the response is the only +/// way this side ever knows when a page dies. +@Suite("page location") +struct PageLocationTests { + private func decode(_ json: String) throws -> PageLocation { + try JSONDecoder.stele.decode(PageLocation.self, from: Data(json.utf8)) + } + + @Test("a deadline is read off the response rather than computed here") + func readsTheDeadline() throws { + let location = try decode( + #"{"slug":"a-b-c","url":"https://s.example.com/a-b-c","expires":"2026-08-12T10:00:00Z"}"# + ) + #expect(location.slug == "a-b-c") + #expect(location.expiresAt == Date(timeIntervalSince1970: 1_786_528_800)) + } + + /// Both wire shapes for "no deadline". An explicit null is what the server sends for a page + /// it will keep; the key is absent from a deployment older than page expiry, where nothing + /// expires either. Reading them the same way is not a shortcut — they mean the same thing. + @Test("null and absent both mean the page has no deadline", arguments: [ + #"{"slug":"a","url":"u","expires":null}"#, + #"{"slug":"a","url":"u"}"#, + ]) + func noDeadline(_ json: String) throws { + #expect(try decode(json).expiresAt == nil) + } + + /// Fractional seconds are what a Postgres `timestamptz` renders as through most encoders, + /// so the deadline has to survive the same shapes every other timestamp does. + @Test("the deadline accepts both ISO 8601 shapes") + func deadlineTimestampShapes() throws { + let plain = try decode(#"{"slug":"a","url":"u","expires":"2026-08-12T10:00:00Z"}"#) + let fractional = try decode(#"{"slug":"a","url":"u","expires":"2026-08-12T10:00:00.123Z"}"#) + #expect(plain.expiresAt != nil) + #expect(fractional.expiresAt != nil) + } + + /// `--json` is a machine contract, and the synthesised encoder would drop the key entirely + /// for a page that is kept — leaving a reader unable to tell "no deadline" from "this tool + /// said nothing about deadlines". The server hand-writes its encoder for this; so does this. + @Test("a kept page encodes an explicit null rather than dropping the key") + func encodesExplicitNull() throws { + let json = String( + decoding: try JSONEncoder().encode(PageLocation(slug: "a", url: "u")), as: UTF8.self + ) + #expect(json.contains("\"expires\":null")) + } + + /// The round trip the `--json` documentation promises: what this tool prints, it can read. + @Test("a location survives the round trip through its own JSON") + func roundTrips() throws { + let encoder = JSONEncoder() + encoder.dateEncodingStrategy = .iso8601 + for expiry in [nil, Date(timeIntervalSince1970: 1_786_528_800)] { + let original = PageLocation(slug: "a-b-c", url: "u", expiresAt: expiry) + let decoded = try JSONDecoder.stele.decode( + PageLocation.self, from: try encoder.encode(original) + ) + #expect(decoded == original) + } + } +} + @Suite("status mapping") struct SteleErrorMappingTests { private func failure(status: Int, body: String = "") async -> (any Error)? { diff --git a/scripts/integration-smoke.sh b/scripts/integration-smoke.sh index 7a2df6f..b73e584 100755 --- a/scripts/integration-smoke.sh +++ b/scripts/integration-smoke.sh @@ -90,6 +90,21 @@ expect_contains() { } # json_field — prints the value, or the empty string for null/absent. +# json_key_state -> "absent", "null", or "set". +# +# `json_field` collapses a null and a missing key to the same empty string, and for `expires` +# that is exactly the distinction worth testing: the synthesised `Encodable` would *drop* the +# key for a page that is kept, leaving a reader unable to tell "no deadline" from "this tool +# has no opinion about deadlines". Both ends hand-write an encoder to say null out loud. +json_key_state() { + python3 - "$1" "$2" <<'PY' +import json, sys +document = json.load(open(sys.argv[1])) +key = sys.argv[2] +print("absent" if key not in document else "null" if document[key] is None else "set") +PY +} + json_field() { python3 - "$1" "$2" <<'PY' import json, sys @@ -127,10 +142,16 @@ PY } # Runs the CLI and captures its exit status without tripping `set -e`-style aborts. -# Output goes to $OUT, status to $STATUS. +# stdout goes to $OUT, stderr to $ERR, status to $STATUS. +# +# The two streams are kept apart rather than merged, because the split is itself a promise this +# tool makes: stdout carries the URL and nothing else, so `url=$(stele publish page.html)` +# works. Merging them here would have hidden a regression that put a second line on stdout. run_stele() { - OUT="$("$STELE_BIN" "$@" --host "$HOST" 2>&1)" + ERR_FILE="$WORK/stderr" + OUT="$("$STELE_BIN" "$@" --host "$HOST" 2>"$ERR_FILE")" STATUS=$? + ERR="$(cat "$ERR_FILE")" } http_status() { curl -sS -o /dev/null -w '%{http_code}' "$@"; } @@ -280,6 +301,68 @@ fi expect_eq "…served as text/html" "text/html; charset=utf-8" \ "$(curl -sS -o /dev/null -w '%{content_type}' "$URL")" +# The deadline is real output and it is on the *other* stream. Both halves matter: a caller +# doing `url=$(stele publish page.html)` must not capture it, and a caller watching the +# terminal must see it — a page published with no --ttl is ephemeral, and being handed a bare +# URL is how you find that out when the link breaks. +expect_contains "…and the deadline is reported on stderr" "expires" "$ERR" +# Counted rather than pattern-matched: `$(printf '\n')` is the empty string — command +# substitution strips trailing newlines — so the obvious `case` spelling asks whether stdout +# contains "", which every string does, and the check passes for one line and fails for ten. +if [ "$(printf '%s' "$OUT" | wc -l)" -eq 0 ]; then + pass "…while stdout stays the URL and nothing else" +else + fail "stdout stays one line" "just the URL" "$OUT" +fi + +# --------------------------------------------------------------------------------- +section "4b. page lifetimes: the default is ephemeral and --ttl is the way out" +# --------------------------------------------------------------------------------- + +# What the server actually stored, not what the CLI printed. `expires` is the server's key and +# an explicit null is its way of saying "kept" — the distinction the CLI's own encoder keeps. +run_stele publish "$WORK/page.html" --json +expect_eq "publish --json exits 0" "0" "$STATUS" +printf '%s' "$OUT" > "$WORK/page-default.json" +DEFAULT_EXPIRY="$(json_field "$WORK/page-default.json" expires)" +expect_eq "a page published with no --ttl is ephemeral by default" "set" \ + "$(json_key_state "$WORK/page-default.json" expires)" +expect_eq "…and --json puts nothing on stderr" "" "$ERR" + +run_stele publish "$WORK/page.html" --ttl never --json +expect_eq "--ttl never exits 0" "0" "$STATUS" +printf '%s' "$OUT" > "$WORK/page-never.json" +expect_eq "…and the page is kept: an explicit null, not a missing key" "null" \ + "$(json_key_state "$WORK/page-never.json" expires)" + +run_stele publish "$WORK/page.html" --ttl 1 +expect_eq "--ttl 1 exits 0" "0" "$STATUS" +expect_contains "…and reports a deadline a day out" "expires" "$ERR" +TTL_URL="$OUT" +expect_eq "…and the page is served now" "200" "$(http_status "$TTL_URL")" + +# A week and a day are different lifetimes, and the CLI must not be quietly sending the same +# thing for both. Compared through the server's own record rather than the printed line. +run_stele publish "$WORK/page.html" --ttl 90 --json +expect_eq "--ttl 90 exits 0" "0" "$STATUS" +printf '%s' "$OUT" > "$WORK/page-90.json" +if [ "$(json_field "$WORK/page-90.json" expires)" != "$DEFAULT_EXPIRY" ]; then + pass "…and --ttl 90 stores a different deadline than the default" +else + fail "--ttl 90 reaches the server" "a deadline unlike the default" "$DEFAULT_EXPIRY" +fi + +# Refused here, before the upload: nothing is rounded, and no page is created. +run_stele publish "$WORK/page.html" --ttl 12h +expect_eq "a sub-day lifetime is refused, exit 1" "1" "$STATUS" +expect_contains "…and the message says days are the resolution" "whole days" "$ERR" +expect_eq "…and nothing was published" "" "$OUT" + +# Refused there, by the server, which owns the maximum. The CLI keeps no copy of that bound, +# so this is the leg that proves an over-long lifetime still fails — with the server's number. +run_stele publish "$WORK/page.html" --ttl 3650000 +expect_eq "a lifetime past the server's ceiling is a 400, exit 1" "1" "$STATUS" + # --------------------------------------------------------------------------------- section "5. update replaces it in place" # --------------------------------------------------------------------------------- @@ -300,6 +383,17 @@ else fail "the replacement is what is served" "the second file" "$(diff "$WORK/page2.html" "$WORK/fetched2.html" | head -5)" fi +# A page's deadline is set once, at publish. Replacing the body must not buy the link another +# week — which is why `update` has no --ttl to offer and the server answers `?ttl=` on PUT +# with a 400 rather than a 200 that ignored it. +expect_contains "…and still reports the deadline it already had" "expires" "$ERR" +run_stele update "$SLUG" "$WORK/page2.html" --ttl never +# 64, not 1: an option the command does not declare is a usage error, and ArgumentParser exits +# `EX_USAGE` for those. The distinction is worth pinning — 1 would mean the flag was accepted +# and something later refused it, which is the outcome this check exists to rule out. +expect_eq "update has no --ttl to give" "64" "$STATUS" +expect_contains "…and says so rather than silently extending the page" "--ttl" "$ERR" + # --------------------------------------------------------------------------------- section "6. --expires-in survives the round trip" # ---------------------------------------------------------------------------------