From f18129ee7893cc6f42831acb637943de72d52d28 Mon Sep 17 00:00:00 2001 From: Forge Workflow Engine Date: Fri, 12 Jun 2026 17:01:07 +0000 Subject: [PATCH 1/7] forge: workflow 99171cbe-7c27-4976-a999-931b834b5d62 run d97cc852-279d-4b65-affb-3d6f61fb8f82 init idempotency: 99171cbe-7c27-4976-a999-931b834b5d62:d97cc852-279d-4b65-affb-3d6f61fb8f82:init --- .../brief.md | 152 ++++++++++++++++++ .../inputs.json | 6 + .../spec.json | 125 ++++++++++++++ 3 files changed, 283 insertions(+) create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/brief.md create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/inputs.json create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/spec.json diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/brief.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/brief.md new file mode 100644 index 00000000..058a6678 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/brief.md @@ -0,0 +1,152 @@ +Bug: `sc secrets add/hide` fails on UTF-8 secret files — "crypto/rsa: message too long for RSA key size" + +## Summary + +`sc secrets add` / `sc secrets hide` fail to encrypt a secret file when the file +contains enough **multi-byte UTF-8** characters (emoji, box-drawing `─`, arrows +`→ ↔`, em-dash `—`, Cyrillic, CJK, accented letters) clustered together. The +command aborts with: + +``` +Error executing command: failed to re-encrypt all secrets: + failed to encrypt secret file: "" with publicKey "ssh-rsa AAAAB3N": + failed to encrypt secret: crypto/rsa: message too long for RSA key size +``` + +The failure is **data-dependent and intermittent**: an ASCII-only file of the +same size encrypts fine; a file with a dense run of multi-byte characters fails. +This makes it confusing to diagnose (it looks like a key-size problem, but all +keys are 2048-bit and the file is far smaller than any RSA limit). + +## Impact + +- Any secret file with non-ASCII content (comments with box-drawing/arrows, + emoji in values, Cyrillic/CJK text) can become un-encryptable. +- Severity: medium. Blocks adding/rotating affected secrets; no data loss. +- Real case: a host_vars file with section-header comments built from `─` and + `→ ↔ —` characters (worst 128-rune window = 260 bytes) failed; the same file + with those replaced by ASCII (`-`, `->`, `<->`, `--`) encrypted fine. + +## Root cause + +`pkg/api/secrets/ciphers/encryption.go`, `EncryptLargeString` (≈ line 147): + +```go +func EncryptLargeString(key crypto.PublicKey, s string) ([]string, error) { + if rsaKey, ok := key.(*rsa.PublicKey); ok { + chunks := lo.ChunkString(s, rsaKey.Size()/2) // <-- (1) chunk size in RUNES + for idx, chunk := range chunks { + encryptedData, err := rsa.EncryptOAEP( + sha256.New(), rand.Reader, rsaKey, []byte(chunk), nil) // <-- (2) OAEP byte limit + ... + } + } +} +``` + +Two compounding issues: + +1. **Chunking is by runes, the limit is in bytes.** `lo.ChunkString` splits the + string by **rune count** (it operates on `[]rune`). The chunk is then converted + back with `[]byte(chunk)`. For multi-byte UTF-8, the byte length of a chunk can + be up to 4× its rune count. + +2. **The chunk size `rsaKey.Size()/2` is not a safe OAEP message size.** + RSA-OAEP can encrypt at most `k − 2·hLen − 2` bytes, where `k` = modulus size + in bytes and `hLen` = hash output size. With SHA-256 (`hLen = 32`) and a + 2048-bit key (`k = 256`): **max = 256 − 64 − 2 = 190 bytes**. + The code uses `rsaKey.Size()/2 = 128` as the chunk size. + + - For **ASCII**: 128 runes = 128 bytes ≤ 190 → OK (which is why it usually works). + - For **multi-byte UTF-8**: a 128-rune chunk can be 129…512 bytes. As soon as + one chunk exceeds 190 bytes, `rsa.EncryptOAEP` returns + `crypto/rsa: message too long for RSA key size`. + +So `Size()/2` only *happens* to be safe for ASCII; it is unsafe in general +because (a) it counts runes not bytes and (b) `Size()/2` > the true OAEP limit +once content is multi-byte. + +### Verification (math + measured) + +- 2048-bit key: OAEP-SHA256 limit = 190 bytes; code chunk size = 128. +- Measured worst-case 128-rune window (bytes): + - ASCII-heavy host_vars: 160 B, 168 B → encrypt OK. + - Box-drawing-heavy host_vars: **260 B** → encrypt FAILS. + +## Reproduction + +```sh +# A file whose 128-rune window exceeds 190 bytes when UTF-8 encoded. +printf '# %s\n' "$(python3 -c 'print("─"*120)')" > /tmp/secret.txt # 120x U+2500 = 360 bytes +sc secrets add /tmp/secret.txt +# -> crypto/rsa: message too long for RSA key size +``` + +## Proposed fix + +Chunk by **bytes** with a size that respects the OAEP limit, and operate on the +byte slice instead of runes. Splitting a multi-byte character across chunk +boundaries is safe here because `DecryptLargeString` concatenates the decrypted +byte chunks back exactly (`strings.Join` of the per-chunk strings reproduces the +original byte sequence; Go strings are byte sequences, so an invalid-UTF-8 +fragment in one chunk is harmless once joined). + +```go +func EncryptLargeString(key crypto.PublicKey, s string) ([]string, error) { + if rsaKey, ok := key.(*rsa.PublicKey); ok { + // RSA-OAEP max message = k - 2*hLen - 2 (SHA-256 => hLen=32). Leave a + // small safety margin. + maxPlain := rsaKey.Size() - 2*sha256.Size() - 2 // 190 for 2048-bit + if maxPlain <= 0 { + return nil, errors.Errorf("RSA key too small (%d bits) for OAEP-SHA256", rsaKey.Size()*8) + } + data := []byte(s) + res := make([]string, 0, (len(data)+maxPlain-1)/maxPlain) + for i := 0; i < len(data); i += maxPlain { + end := i + maxPlain + if end > len(data) { + end = len(data) + } + enc, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, data[i:end], nil) + if err != nil { + return nil, errors.Wrap(err, "failed to encrypt secret") + } + res = append(res, base64.StdEncoding.EncodeToString(enc)) + } + return res, nil + } + // ... ed25519 branch unchanged ... +} +``` + +`DecryptLargeString` needs no change (it already decodes + RSA-decrypts each +chunk and joins the resulting bytes). + +### Backward compatibility + +Existing secrets stored with the old rune-based chunking still decrypt correctly +(decryption is chunk-list driven and chunk-size agnostic). Only **re-encryption** +(add/hide) produces the new byte-based chunks. A `--force` re-encrypt of the whole +store after the fix is safe. + +### Alternative (recommended longer term) + +Switch the RSA path to **hybrid encryption**, mirroring the existing ed25519 +path (`encryptWithEd25519`): generate a random symmetric key, encrypt the file +once with AES-GCM / ChaCha20-Poly1305, and RSA-OAEP-encrypt only the small +symmetric key. This eliminates chunking entirely, is faster, and removes this +whole class of size bug. + +## Secondary note (latent inconsistency) + +`EncryptWithPublicRSAKey` (same file, ≈ line 86) does a **single-shot** +`rsa.EncryptOAEP` with **SHA-512** (`hLen=64` → max 126 bytes for a 2048-bit key) +and no chunking, whereas `EncryptLargeString` uses **SHA-256** + chunking. If any +caller routes large data through `EncryptWithPublicRSAKey`, it will fail for +anything > 126 bytes. Consider standardizing the hash and always going through +the chunked/hybrid path. + +## Affected file + +- `pkg/api/secrets/ciphers/encryption.go` — `EncryptLargeString` (chunking), + and `EncryptWithPublicRSAKey` (hash inconsistency). \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/inputs.json b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/inputs.json new file mode 100644 index 00000000..e3df13d4 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/inputs.json @@ -0,0 +1,6 @@ +{ + "brief": "Bug: `sc secrets add/hide` fails on UTF-8 secret files — \"crypto/rsa: message too long for RSA key size\"\n\n## Summary\n\n`sc secrets add` / `sc secrets hide` fail to encrypt a secret file when the file\ncontains enough **multi-byte UTF-8** characters (emoji, box-drawing `─`, arrows\n`→ ↔`, em-dash `—`, Cyrillic, CJK, accented letters) clustered together. The\ncommand aborts with:\n\n```\nError executing command: failed to re-encrypt all secrets:\n failed to encrypt secret file: \"\u003cpath\u003e\" with publicKey \"ssh-rsa AAAAB3N\":\n failed to encrypt secret: crypto/rsa: message too long for RSA key size\n```\n\nThe failure is **data-dependent and intermittent**: an ASCII-only file of the\nsame size encrypts fine; a file with a dense run of multi-byte characters fails.\nThis makes it confusing to diagnose (it looks like a key-size problem, but all\nkeys are 2048-bit and the file is far smaller than any RSA limit).\n\n## Impact\n\n- Any secret file with non-ASCII content (comments with box-drawing/arrows,\n emoji in values, Cyrillic/CJK text) can become un-encryptable.\n- Severity: medium. Blocks adding/rotating affected secrets; no data loss.\n- Real case: a host_vars file with section-header comments built from `─` and\n `→ ↔ —` characters (worst 128-rune window = 260 bytes) failed; the same file\n with those replaced by ASCII (`-`, `-\u003e`, `\u003c-\u003e`, `--`) encrypted fine.\n\n## Root cause\n\n`pkg/api/secrets/ciphers/encryption.go`, `EncryptLargeString` (≈ line 147):\n\n```go\nfunc EncryptLargeString(key crypto.PublicKey, s string) ([]string, error) {\n if rsaKey, ok := key.(*rsa.PublicKey); ok {\n chunks := lo.ChunkString(s, rsaKey.Size()/2) // \u003c-- (1) chunk size in RUNES\n for idx, chunk := range chunks {\n encryptedData, err := rsa.EncryptOAEP(\n sha256.New(), rand.Reader, rsaKey, []byte(chunk), nil) // \u003c-- (2) OAEP byte limit\n ...\n }\n }\n}\n```\n\nTwo compounding issues:\n\n1. **Chunking is by runes, the limit is in bytes.** `lo.ChunkString` splits the\n string by **rune count** (it operates on `[]rune`). The chunk is then converted\n back with `[]byte(chunk)`. For multi-byte UTF-8, the byte length of a chunk can\n be up to 4× its rune count.\n\n2. **The chunk size `rsaKey.Size()/2` is not a safe OAEP message size.**\n RSA-OAEP can encrypt at most `k − 2·hLen − 2` bytes, where `k` = modulus size\n in bytes and `hLen` = hash output size. With SHA-256 (`hLen = 32`) and a\n 2048-bit key (`k = 256`): **max = 256 − 64 − 2 = 190 bytes**.\n The code uses `rsaKey.Size()/2 = 128` as the chunk size.\n\n - For **ASCII**: 128 runes = 128 bytes ≤ 190 → OK (which is why it usually works).\n - For **multi-byte UTF-8**: a 128-rune chunk can be 129…512 bytes. As soon as\n one chunk exceeds 190 bytes, `rsa.EncryptOAEP` returns\n `crypto/rsa: message too long for RSA key size`.\n\nSo `Size()/2` only *happens* to be safe for ASCII; it is unsafe in general\nbecause (a) it counts runes not bytes and (b) `Size()/2` \u003e the true OAEP limit\nonce content is multi-byte.\n\n### Verification (math + measured)\n\n- 2048-bit key: OAEP-SHA256 limit = 190 bytes; code chunk size = 128.\n- Measured worst-case 128-rune window (bytes):\n - ASCII-heavy host_vars: 160 B, 168 B → encrypt OK.\n - Box-drawing-heavy host_vars: **260 B** → encrypt FAILS.\n\n## Reproduction\n\n```sh\n# A file whose 128-rune window exceeds 190 bytes when UTF-8 encoded.\nprintf '# %s\\n' \"$(python3 -c 'print(\"─\"*120)')\" \u003e /tmp/secret.txt # 120x U+2500 = 360 bytes\nsc secrets add /tmp/secret.txt\n# -\u003e crypto/rsa: message too long for RSA key size\n```\n\n## Proposed fix\n\nChunk by **bytes** with a size that respects the OAEP limit, and operate on the\nbyte slice instead of runes. Splitting a multi-byte character across chunk\nboundaries is safe here because `DecryptLargeString` concatenates the decrypted\nbyte chunks back exactly (`strings.Join` of the per-chunk strings reproduces the\noriginal byte sequence; Go strings are byte sequences, so an invalid-UTF-8\nfragment in one chunk is harmless once joined).\n\n```go\nfunc EncryptLargeString(key crypto.PublicKey, s string) ([]string, error) {\n if rsaKey, ok := key.(*rsa.PublicKey); ok {\n // RSA-OAEP max message = k - 2*hLen - 2 (SHA-256 =\u003e hLen=32). Leave a\n // small safety margin.\n maxPlain := rsaKey.Size() - 2*sha256.Size() - 2 // 190 for 2048-bit\n if maxPlain \u003c= 0 {\n return nil, errors.Errorf(\"RSA key too small (%d bits) for OAEP-SHA256\", rsaKey.Size()*8)\n }\n data := []byte(s)\n res := make([]string, 0, (len(data)+maxPlain-1)/maxPlain)\n for i := 0; i \u003c len(data); i += maxPlain {\n end := i + maxPlain\n if end \u003e len(data) {\n end = len(data)\n }\n enc, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, data[i:end], nil)\n if err != nil {\n return nil, errors.Wrap(err, \"failed to encrypt secret\")\n }\n res = append(res, base64.StdEncoding.EncodeToString(enc))\n }\n return res, nil\n }\n // ... ed25519 branch unchanged ...\n}\n```\n\n`DecryptLargeString` needs no change (it already decodes + RSA-decrypts each\nchunk and joins the resulting bytes).\n\n### Backward compatibility\n\nExisting secrets stored with the old rune-based chunking still decrypt correctly\n(decryption is chunk-list driven and chunk-size agnostic). Only **re-encryption**\n(add/hide) produces the new byte-based chunks. A `--force` re-encrypt of the whole\nstore after the fix is safe.\n\n### Alternative (recommended longer term)\n\nSwitch the RSA path to **hybrid encryption**, mirroring the existing ed25519\npath (`encryptWithEd25519`): generate a random symmetric key, encrypt the file\nonce with AES-GCM / ChaCha20-Poly1305, and RSA-OAEP-encrypt only the small\nsymmetric key. This eliminates chunking entirely, is faster, and removes this\nwhole class of size bug.\n\n## Secondary note (latent inconsistency)\n\n`EncryptWithPublicRSAKey` (same file, ≈ line 86) does a **single-shot**\n`rsa.EncryptOAEP` with **SHA-512** (`hLen=64` → max 126 bytes for a 2048-bit key)\nand no chunking, whereas `EncryptLargeString` uses **SHA-256** + chunking. If any\ncaller routes large data through `EncryptWithPublicRSAKey`, it will fail for\nanything \u003e 126 bytes. Consider standardizing the hash and always going through\nthe chunked/hybrid path.\n\n## Affected file\n\n- `pkg/api/secrets/ciphers/encryption.go` — `EncryptLargeString` (chunking),\n and `EncryptWithPublicRSAKey` (hash inconsistency).", + "issueId": "6a2c25990ce79d1156204861", + "issueNumber": 318, + "issueRepo": "simple-container-com/api" +} \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/spec.json b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/spec.json new file mode 100644 index 00000000..ded1c416 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/spec.json @@ -0,0 +1,125 @@ +{ + "initialCharacterId": "pm", + "characters": [ + { + "id": "pm", + "agentId": "34890141-d95a-4628-9507-35a06c70827e", + "displayName": "Max Warner", + "roleHint": "pm", + "capabilities": { + "hasDocker": false, + "hasPersistentDisk": false, + "maxTurnDurationSec": 1800, + "supportsLongLived": false, + "isolation": "process", + "egressPolicy": "scoped", + "minMemoryMB": 512 + }, + "description": "Product Manager — Max Warner (autonomous slice-id addition shipped 2026-06-11)." + }, + { + "id": "architect", + "agentId": "7b6ae717-cab4-4b7c-b4ea-ed61353ee6f4", + "displayName": "Dan Johnson", + "roleHint": "architect", + "capabilities": { + "hasDocker": false, + "hasPersistentDisk": false, + "maxTurnDurationSec": 1800, + "supportsLongLived": false, + "isolation": "process", + "egressPolicy": "scoped", + "minMemoryMB": 512 + }, + "description": "Architect — Dan Johnson (no prompt changes for SDLC; existing persona is strong)." + }, + { + "id": "developer", + "agentId": "0d913379-fa6c-4833-8c61-b39a94acec46", + "displayName": "David Black", + "roleHint": "developer", + "capabilities": { + "hasDocker": true, + "hasPersistentDisk": true, + "maxTurnDurationSec": 3600, + "supportsLongLived": false, + "isolation": "container", + "egressPolicy": "open", + "minMemoryMB": 2048, + "runtimeKindsAllowed": [ + "container", + "vm" + ] + }, + "description": "Developer — David Black (autonomous workflow-branch commit clause shipped 2026-06-11)." + }, + { + "id": "qa", + "agentId": "b49e743d-1004-406c-b832-db4098eaa8ac", + "displayName": "Maria Currie", + "roleHint": "qa", + "capabilities": { + "hasDocker": true, + "hasPersistentDisk": false, + "maxTurnDurationSec": 2700, + "supportsLongLived": false, + "isolation": "container", + "egressPolicy": "open", + "minMemoryMB": 1024, + "runtimeKindsAllowed": [ + "container", + "vm" + ] + }, + "description": "QA — Maria Currie (autonomous deploy/smoke/destroy/sign-off duties block shipped 2026-06-11)." + }, + { + "id": "devops", + "agentId": "7075eb90-38ec-4ab2-8159-c9eb8a2ff5a5", + "displayName": "William Smith", + "roleHint": "devops", + "capabilities": { + "hasDocker": false, + "hasPersistentDisk": false, + "maxTurnDurationSec": 1800, + "supportsLongLived": false, + "isolation": "process", + "egressPolicy": "scoped", + "minMemoryMB": 512 + }, + "description": "DevOps — William Smith (autonomous chain-continuation tail shipped 2026-06-11)." + } + ], + "transitions": [ + { + "id": "pm-to-architect", + "fromCharacterId": "pm", + "toCharacterId": "architect", + "escalationPolicy": "operator_review" + }, + { + "id": "architect-to-developer", + "fromCharacterId": "architect", + "toCharacterId": "developer", + "escalationPolicy": "operator_review" + }, + { + "id": "developer-to-qa", + "fromCharacterId": "developer", + "toCharacterId": "qa", + "escalationPolicy": "operator_review" + }, + { + "id": "qa-to-devops", + "fromCharacterId": "qa", + "toCharacterId": "devops", + "condition": "handoff.verdict == 'signoff'", + "escalationPolicy": "operator_review" + } + ], + "managementAgentId": "architect", + "enrichBudgetDefault": 0, + "storageMode": "shared", + "allowWorkflowTriggers": true, + "autonomyBudget": 3 +} \ No newline at end of file From b55f05b234e46ab637904cc5571419a48b2cabb3 Mon Sep 17 00:00:00 2001 From: "Max Warner (pm)" Date: Fri, 12 Jun 2026 17:02:27 +0000 Subject: [PATCH 2/7] forge: character pm (Max Warner) phase run idempotency: pm:run Co-authored-by: Max Warner (pm) --- .../agents/pm/handoff.json | 8 + .../agents/pm/handoff.md | 187 ++++++++++++++++++ .../agents/pm/summary.md | 1 + 3 files changed, 196 insertions(+) create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.json create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.md create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/summary.md diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.json b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.json new file mode 100644 index 00000000..08992b7e --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.json @@ -0,0 +1,8 @@ +{ + "bodyBytes": 7804, + "characterId": "pm", + "displayName": "Max Warner", + "phase": "run", + "roleHint": "pm", + "writtenAt": "2026-06-12T17:02:25.20257955Z" +} \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.md new file mode 100644 index 00000000..352cab4a --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/handoff.md @@ -0,0 +1,187 @@ +I have everything I need. Let me now produce the PM handoff document. + +--- + +# PM Handoff — Run `d97cc852-279d-4b65-affb-3d6f61fb8f82` +**Date:** 2026-06-12 +**Workflow repo:** `simple-container-com/api` · branch `forge/workflow/99171cbe-7c27-4976-a999-931b834b5d62/run/d97cc852-279d-4b65-affb-3d6f61fb8f82` +**Character:** Max Warner (PM) + +--- + +## 1 — Bug Statement (one line) + +`EncryptLargeString` chunks the plaintext by **rune count** against a limit that is in **bytes**, causing `rsa.EncryptOAEP` to fail with "message too long" on any secret file containing multi-byte UTF-8 characters. + +--- + +## 2 — Scope Decision + +| # | Item | Label | Rationale | +|---|------|-------|-----------| +| 1 | Fix `EncryptLargeString`: chunk by bytes, use correct OAEP-SHA256 max (`k − 2·hLen − 2 = 190` for 2048-bit) | **MUST** | Direct root cause; one function, ~10 lines | +| 2 | Drop `lo.ChunkString` dependency from the RSA path | **MUST** | Required by fix; rune-based chunking is the bug | +| 3 | Add failing regression test: RSA encrypt/decrypt of 128-rune UTF-8 string (box-drawing `─` × 120) | **MUST** | Proves the bug is fixed; must be committed with the fix | +| 4 | Add note / `//nolint` on `EncryptWithPublicRSAKey`'s SHA-512 inconsistency | **NICE-TO-HAVE** | Latent inconsistency; no known crash path today; document it, don't fix it in this PR | +| 5 | Migrate RSA path to hybrid encryption (AES-GCM / ChaCha20) | **CUT** | Correct long-term direction but out of scope for a bug fix; schedule separately | +| 6 | Force-re-encrypt all existing secrets in stores | **CUT** | Operational concern for users; backward compat is guaranteed without it | + +--- + +## 3 — Exact Change Required + +### File: `pkg/api/secrets/ciphers/encryption.go` + +**Replace** the RSA branch inside `EncryptLargeString` (currently ≈ lines 147–159): + +```go +// BEFORE (buggy) +if rsaKey, ok := key.(*rsa.PublicKey); ok { + chunks := lo.ChunkString(s, rsaKey.Size()/2) // rune-count chunk — WRONG + res = make([]string, len(chunks)) + for idx, chunk := range chunks { + encryptedData, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, []byte(chunk), nil) + ... + res[idx] = base64.StdEncoding.EncodeToString(encryptedData) + } +} +``` + +```go +// AFTER (correct) +if rsaKey, ok := key.(*rsa.PublicKey); ok { + // RSA-OAEP max plaintext = k − 2·hLen − 2. + // SHA-256 → hLen=32; 2048-bit key → k=256; max = 190 bytes. + maxPlain := rsaKey.Size() - 2*sha256.Size - 2 + if maxPlain <= 0 { + return nil, errors.Errorf("RSA key too small (%d bits) for OAEP-SHA256", rsaKey.Size()*8) + } + data := []byte(s) + res = make([]string, 0, (len(data)+maxPlain-1)/maxPlain) + for i := 0; i < len(data); i += maxPlain { + end := i + maxPlain + if end > len(data) { + end = len(data) + } + enc, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, data[i:end], nil) + if err != nil { + return nil, errors.Wrap(err, "failed to encrypt secret") + } + res = append(res, base64.StdEncoding.EncodeToString(enc)) + } +} +``` + +**Import cleanup:** remove `"github.com/samber/lo"` if it is no longer used elsewhere in the file (check: `lo.Map` is still used in `DecryptLargeString` — keep the import, just confirm `lo.ChunkString` is the only removed call). + +> Note on `sha256.Size`: the `crypto/sha256` package exports the constant `sha256.Size = 32`. Use that rather than a magic number. + +--- + +## 4 — Test Required (regression gate) + +### File: `pkg/api/secrets/ciphers/encryption_test.go` (or a new `encryption_utf8_test.go`) + +Add the following test cases inside the existing `TestRSAEncryptionDecryption` suite **or** as a new top-level function: + +```go +t.Run("RSA encrypt/decrypt UTF-8 multi-byte string (box-drawing)", func(t *testing.T) { + // U+2500 BOX DRAWINGS LIGHT HORIZONTAL = 3 bytes in UTF-8. + // 120 runes × 3 bytes = 360 bytes > old 128-rune chunk limit (was failing). + utf8Heavy := strings.Repeat("─", 120) // 360 bytes + + encryptedChunks, err := EncryptLargeString(pubKey, utf8Heavy) + Expect(err).To(BeNil()) + Expect(encryptedChunks).NotTo(BeEmpty()) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(utf8Heavy)) +}) + +t.Run("RSA encrypt/decrypt emoji-dense string", func(t *testing.T) { + // U+1F600 GRINNING FACE = 4 bytes. 100 runes = 400 bytes. + emojiHeavy := strings.Repeat("😀", 100) // 400 bytes + + encryptedChunks, err := EncryptLargeString(pubKey, emojiHeavy) + Expect(err).To(BeNil()) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(emojiHeavy)) +}) + +t.Run("RSA chunk boundaries do not corrupt multi-byte sequences", func(t *testing.T) { + // Fill exactly 2 chunks worth of 3-byte runes; boundary falls mid-sequence. + // 190 bytes per chunk × 2 = 380 bytes needed; 127 × 3 = 381 bytes. + crossBoundary := strings.Repeat("─", 127) // 381 bytes → spans 2 chunks + + encryptedChunks, err := EncryptLargeString(pubKey, crossBoundary) + Expect(err).To(BeNil()) + Expect(len(encryptedChunks)).To(Equal(2)) // must produce exactly 2 chunks + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(crossBoundary)) +}) +``` + +--- + +## 5 — Acceptance Criteria (definition of done) + +| # | Criterion | +|---|-----------| +| AC-1 | `go test ./pkg/api/secrets/ciphers/...` passes, including all new UTF-8 cases | +| AC-2 | The three new test cases each fail on the old code (verified by reverting the fix temporarily or by inspection of the math) | +| AC-3 | Existing `TestRSAEncryptionDecryption / RSA encrypt/decrypt large string` still passes (backward compat) | +| AC-4 | `lo.ChunkString` is removed from the RSA encryption path | +| AC-5 | No new external dependencies introduced | + +--- + +## 6 — Critical-Path Sequence + +``` +1. Engineer applies fix to EncryptLargeString (pkg/api/secrets/ciphers/encryption.go) +2. Engineer adds failing-then-passing regression tests +3. go test ./pkg/api/secrets/ciphers/... — must be green +4. PR opened; AC-1 through AC-5 verified in CI +5. Merge → done +``` + +No design doc, Swagger regen, or roadmap update required (this is a bug fix in an internal cipher utility, not a public API or roadmap feature). + +--- + +## 7 — Secondary Note (do NOT block merge on this) + +`EncryptWithPublicRSAKey` uses SHA-512 (hLen=64 → max 126 B for 2048-bit) while `EncryptLargeString` uses SHA-256 (hLen=32 → max 190 B). These are inconsistent. Add a `// TODO:` comment noting this. Do **not** change `EncryptWithPublicRSAKey`'s hash in this PR — that would break existing encrypted payloads that use it. + +--- + +## 8 — Handoff JSON + +```json +{ + "run_id": "d97cc852-279d-4b65-affb-3d6f61fb8f82", + "from_character": "pm", + "to_character": "engineer", + "date": "2026-06-12", + "bug": "EncryptLargeString RSA path chunks by rune count, overflows OAEP byte limit on UTF-8 input", + "affected_file": "pkg/api/secrets/ciphers/encryption.go", + "affected_function": "EncryptLargeString", + "fix_summary": "Replace lo.ChunkString(s, rsaKey.Size()/2) rune-chunking with byte-slice chunking using maxPlain = rsaKey.Size() - 2*sha256.Size - 2", + "test_file": "pkg/api/secrets/ciphers/encryption_test.go", + "new_test_cases": [ + "RSA encrypt/decrypt UTF-8 multi-byte string (box-drawing)", + "RSA encrypt/decrypt emoji-dense string", + "RSA chunk boundaries do not corrupt multi-byte sequences" + ], + "must_items": [1, 2, 3], + "cut_items": [5, 6], + "acceptance_criteria": ["AC-1", "AC-2", "AC-3", "AC-4", "AC-5"], + "no_new_deps": true, + "backward_compatible": true +} +``` \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/summary.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/summary.md new file mode 100644 index 00000000..e0d1d401 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/pm/summary.md @@ -0,0 +1 @@ +I have everything I need. Let me now produce the PM handoff document. \ No newline at end of file From febb20515e6e4d2197ce1242e41aa214fb91a557 Mon Sep 17 00:00:00 2001 From: "Dan Johnson (architect)" Date: Fri, 12 Jun 2026 17:04:11 +0000 Subject: [PATCH 3/7] forge: character architect (Dan Johnson) phase run idempotency: architect:run Co-authored-by: Dan Johnson (architect) --- .../agents/architect/handoff.json | 8 + .../agents/architect/handoff.md | 251 ++++++++++++++++++ .../agents/architect/summary.md | 1 + 3 files changed, 260 insertions(+) create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.json create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.md create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/summary.md diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.json b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.json new file mode 100644 index 00000000..8128f6e8 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.json @@ -0,0 +1,8 @@ +{ + "bodyBytes": 11727, + "characterId": "architect", + "displayName": "Dan Johnson", + "phase": "run", + "roleHint": "architect", + "writtenAt": "2026-06-12T17:04:09.565158411Z" +} \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.md new file mode 100644 index 00000000..d1c2903a --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/handoff.md @@ -0,0 +1,251 @@ +Good — I have everything I need. I've read the actual source code and tests. No CLAUDE.md exists in this repo. Let me now produce the architectural review and handoff. + +--- + +# Architect Handoff — Run `d97cc852-279d-4b65-affb-3d6f61fb8f82` +**Date:** 2026-06-12 +**From:** Dan Johnson (architect) +**To:** engineer +**Repo:** `simple-container-com/api` · branch `forge/workflow/99171cbe-7c27-4976-a999-931b834b5d62/run/d97cc852-279d-4b65-affb-3d6f61fb8f82` + +--- + +## 1 — Source Review: What I Actually Read + +I fetched and read both files directly from the branch. Key observations: + +### `pkg/api/secrets/ciphers/encryption.go` (current buggy state) + +```go +func EncryptLargeString(key crypto.PublicKey, s string) ([]string, error) { + var res []string + if rsaKey, ok := key.(*rsa.PublicKey); ok { + chunks := lo.ChunkString(s, rsaKey.Size()/2) // BUG: rune-count, not bytes + res = make([]string, len(chunks)) + for idx, chunk := range chunks { + encryptedData, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, []byte(chunk), nil) + ... + } + } +``` + +**Confirmed** — `lo.ChunkString` operates on `[]rune`. For a 2048-bit key: `rsaKey.Size() = 256`, so chunk size = 128 runes. A 128-rune chunk of 3-byte UTF-8 characters = 384 bytes — well above OAEP-SHA256's 190-byte limit. The bug is exactly as described. + +**Also confirmed** — `lo.Map` **is** used in `DecryptLargeString` (line: `lo.Map(decrChunks, func(chunk []byte, _ int) string {...})`), so the `lo` import **must be retained** after the fix. + +### `pkg/api/secrets/ciphers/encryption_test.go` (current test state) + +Existing test `TestRSAEncryptionDecryption / RSA encrypt/decrypt large string` uses `strings.Repeat(testData, 20)` where `testData = "Hello, World! This is a test message for RSA encryption."` — entirely ASCII. This **will not** catch the UTF-8 bug. The regression tests PM specified are necessary and correct. + +--- + +## 2 — Architectural Decision Record + +### ADR-001: Chunk by bytes, not runes + +**Decision:** Replace `lo.ChunkString(s, rsaKey.Size()/2)` with a manual byte-slice loop using `maxPlain = rsaKey.Size() - 2*sha256.Size - 2`. + +**Why this and not alternatives:** + +| Option | Verdict | Reason | +|--------|---------|--------| +| Fix chunk size to bytes with correct OAEP limit | **ADOPT** | Minimal change, correct, backward-compatible decryption | +| Switch to hybrid encryption (RSA wraps AES key) | **DEFER** | Correct long-term, but touches decryption path, wire format, and existing stored secrets. Out of scope for a bug fix. | +| Increase key size | **REJECT** | Does not fix the root bug; just widens the window of failure | +| Pin chunk size to 126 bytes to match EncryptWithPublicRSAKey's SHA-512 limit | **REJECT** | Inconsistency in hash algo is a separate problem; do not conflate | + +**Key math (must be exactly this — do not vary):** +- `k = rsaKey.Size()` = 256 bytes for 2048-bit key +- `hLen = sha256.Size` = 32 (stdlib constant from `crypto/sha256`) +- `maxPlain = k - 2*hLen - 2` = 256 - 64 - 2 = **190 bytes** +- Old code: 128 runes (safe only for ASCII, fails for multi-byte UTF-8) + +**Backward compatibility:** Decryption is chunk-list driven and chunk-size agnostic — `DecryptLargeString` iterates whatever slices it receives. Old ciphertext (rune-chunked) still decrypts correctly. ✓ + +### ADR-002: Keep `lo` import + +`lo.Map` is still used in `DecryptLargeString`. Only `lo.ChunkString` is removed from the RSA encryption path. **Do not remove the import.** + +### ADR-003: No changes to `DecryptLargeString` + +The decryption path is correct as-is. It uses byte-transparent `strings.Join` semantics — splitting at arbitrary byte offsets during encryption is safe because the join reassembles the original byte sequence. + +### ADR-004: TODO comment on SHA inconsistency, not a fix + +`EncryptWithPublicRSAKey` uses SHA-512 (max 126 B for 2048-bit), `EncryptLargeString` uses SHA-256 (max 190 B). This inconsistency is latent — changing `EncryptWithPublicRSAKey`'s hash would break existing SHA-512-encrypted payloads. Add a `// TODO:` comment only. + +--- + +## 3 — Precise Implementation Spec for Engineer + +### File: `pkg/api/secrets/ciphers/encryption.go` + +**Change only the RSA branch of `EncryptLargeString`.** Do not touch `DecryptLargeString`, `EncryptWithPublicRSAKey`, or any ed25519 functions. + +```go +// REMOVE this entire RSA block: +if rsaKey, ok := key.(*rsa.PublicKey); ok { + chunks := lo.ChunkString(s, rsaKey.Size()/2) + res = make([]string, len(chunks)) + for idx, chunk := range chunks { + encryptedData, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, []byte(chunk), nil) + if err != nil { + return nil, errors.Wrapf(err, "failed to encrypt secret") + } + res[idx] = base64.StdEncoding.EncodeToString(encryptedData) + } +} + +// REPLACE WITH: +if rsaKey, ok := key.(*rsa.PublicKey); ok { + // RSA-OAEP with SHA-256: max plaintext = k − 2·hLen − 2 bytes. + // For a 2048-bit key: 256 − 64 − 2 = 190 bytes. + // Chunking by rune count (old behaviour) is unsafe for multi-byte UTF-8: + // a 128-rune chunk can be up to 512 bytes, exceeding the 190-byte OAEP limit. + maxPlain := rsaKey.Size() - 2*sha256.Size - 2 + if maxPlain <= 0 { + return nil, errors.Errorf("RSA key too small (%d bits) for OAEP-SHA256", rsaKey.Size()*8) + } + data := []byte(s) + res = make([]string, 0, (len(data)+maxPlain-1)/maxPlain) + for i := 0; i < len(data); i += maxPlain { + end := i + maxPlain + if end > len(data) { + end = len(data) + } + enc, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, data[i:end], nil) + if err != nil { + return nil, errors.Wrap(err, "failed to encrypt secret") + } + res = append(res, base64.StdEncoding.EncodeToString(enc)) + } +} +``` + +**Also add a TODO to `EncryptWithPublicRSAKey`:** + +```go +// EncryptWithPublicRSAKey encrypts data with public key +// TODO: This uses SHA-512 (max plaintext 126 B for 2048-bit keys) while +// EncryptLargeString uses SHA-256 (max 190 B). Standardize these in a future PR; +// changing the hash here would break decryption of existing payloads. +func EncryptWithPublicRSAKey(msg []byte, pub *rsa.PublicKey) ([]byte, error) { +``` + +**Import verification:** `lo` must remain (still used in `DecryptLargeString`). No new imports needed — `sha256.Size` is already available via the existing `"crypto/sha256"` import. + +--- + +## 4 — Test Spec for Engineer + +### File: `pkg/api/secrets/ciphers/encryption_test.go` + +Add inside or alongside the `TestRSAEncryptionDecryption` function. The key pair (`privKey`, `pubKey`) is already generated at the top of that function — reuse it. The test must use `RegisterTestingT(t)` and Gomega matchers to match the existing style. + +```go +t.Run("RSA encrypt/decrypt UTF-8 multi-byte string (box-drawing)", func(t *testing.T) { + // U+2500 BOX DRAWINGS LIGHT HORIZONTAL = 3 bytes UTF-8. + // 120 runes × 3 bytes = 360 bytes. + // Old rune-based chunking (128 runes/chunk) produced a 360-byte chunk > 190 B → FAIL. + utf8Heavy := strings.Repeat("─", 120) + + encryptedChunks, err := EncryptLargeString(pubKey, utf8Heavy) + Expect(err).To(BeNil()) + Expect(encryptedChunks).NotTo(BeEmpty()) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(utf8Heavy)) +}) + +t.Run("RSA encrypt/decrypt emoji-dense string", func(t *testing.T) { + // U+1F600 GRINNING FACE = 4 bytes UTF-8. 100 runes = 400 bytes. + emojiHeavy := strings.Repeat("😀", 100) + + encryptedChunks, err := EncryptLargeString(pubKey, emojiHeavy) + Expect(err).To(BeNil()) + Expect(encryptedChunks).NotTo(BeEmpty()) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(emojiHeavy)) +}) + +t.Run("RSA chunk boundaries do not corrupt multi-byte sequences", func(t *testing.T) { + // 127 × 3 bytes = 381 bytes → forces exactly 2 chunks (190 + 191). + // Byte boundary at 190 falls mid-rune (U+2500 is bytes E2 94 80). + // Correct decryption proves byte-boundary splitting + join works. + crossBoundary := strings.Repeat("─", 127) + + encryptedChunks, err := EncryptLargeString(pubKey, crossBoundary) + Expect(err).To(BeNil()) + Expect(len(encryptedChunks)).To(Equal(2)) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(crossBoundary)) +}) +``` + +**Chunk-count math for the boundary test:** +- `crossBoundary` = 381 bytes +- chunk 1: bytes [0:190] = 190 bytes +- chunk 2: bytes [190:381] = 191 bytes +- → exactly 2 chunks ✓ + +--- + +## 5 — Acceptance Checklist + +| # | Check | How to verify | +|---|-------|--------------| +| AC-1 | `go test ./pkg/api/secrets/ciphers/...` passes green | Run locally + CI | +| AC-2 | New UTF-8 tests would FAIL on old code | Confirm by math (128-rune chunk = 384 B > 190 B limit) or revert temporarily | +| AC-3 | Existing `RSA encrypt/decrypt large string` still passes | Included in `go test` above | +| AC-4 | `lo.ChunkString` removed from encryption path | `grep -n ChunkString` returns 0 hits | +| AC-5 | `lo` import retained | `DecryptLargeString` still uses `lo.Map` | +| AC-6 | No new imports or dependencies | `go mod tidy` produces no changes | +| AC-7 | TODO comment added to `EncryptWithPublicRSAKey` | Code review | + +--- + +## 6 — Structural Concerns / Watch-outs + +1. **`sha256.Size` not `sha256.Size()`** — It's a constant (`= 32`), not a function. Using `sha256.New().Size()` also works but is verbose. `sha256.Size` is cleaner and idiomatic. + +2. **`errors.Wrap` vs `errors.Wrapf`** — The existing code uses `errors.Wrapf`. The fix uses `errors.Wrap` for the encrypt error (no format args needed) and `errors.Errorf` for the key-too-small guard. Both are from `"github.com/pkg/errors"` which is already imported. Keep consistent. + +3. **Don't touch `DecryptLargeString`** — The `lo.Map` there is fine. The byte-join semantics are correct. No change needed. + +4. **Test registration** — The existing `TestRSAEncryptionDecryption` calls `RegisterTestingT(t)` at the top. Sub-tests inherit that registration. No additional `RegisterTestingT` calls needed inside `t.Run` blocks. + +5. **Chunk-count assertion** — The `Expect(len(encryptedChunks)).To(Equal(2))` assertion is deterministic: 381 bytes / 190 = 2 chunks (190 + 191). This will not flake. + +--- + +## 7 — What This Fix Does NOT Change + +- Wire format for **decryption** — existing secrets remain decryptable +- The `ed25519` path — hybrid encryption is already correct; do not touch +- `EncryptWithPublicRSAKey` / `DecryptWithPrivateRSAKey` — SHA-512, single-shot, documented with TODO only +- Any public API, CLI flags, or command signatures +- No new dependencies, no go.mod changes + +--- + +## 8 — Summary for Engineer + +**Two files to touch, one structural change each:** + +1. **`pkg/api/secrets/ciphers/encryption.go`** + - Replace RSA branch of `EncryptLargeString` (byte-slice loop, `maxPlain = rsaKey.Size() - 2*sha256.Size - 2`) + - Add `// TODO:` comment to `EncryptWithPublicRSAKey` documenting SHA inconsistency + - Verify `lo` import is retained (it is, via `DecryptLargeString`) + +2. **`pkg/api/secrets/ciphers/encryption_test.go`** + - Add three sub-tests inside/adjacent to `TestRSAEncryptionDecryption`: + - box-drawing (360 B, must succeed) + - emoji (400 B, must succeed) + - cross-boundary (381 B, must produce exactly 2 chunks and round-trip correctly) + +**Run `go test ./pkg/api/secrets/ciphers/...` before handing off — it must be green.** \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/summary.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/summary.md new file mode 100644 index 00000000..a798b036 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/architect/summary.md @@ -0,0 +1 @@ +Good — I have everything I need. I've read the actual source code and tests. No CLAUDE.md exists in this repo. Let me now produce the architectural review and handoff. \ No newline at end of file From e120d5f3dc9d1b5a913deec3409b29fe97013a94 Mon Sep 17 00:00:00 2001 From: David Black Date: Fri, 12 Jun 2026 17:07:21 +0000 Subject: [PATCH 4/7] fix(ciphers): chunk RSA plaintext by bytes, not runes, to prevent OAEP overflow on UTF-8 input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EncryptLargeString was splitting the plaintext using lo.ChunkString, which operates on rune count. For a 2048-bit key the chunk size was 128 runes — safe for ASCII (128 bytes ≤ 190-byte OAEP-SHA256 limit) but wrong for multi-byte UTF-8: a 128-rune box-drawing string encodes to 384 bytes, blowing past the limit and producing "crypto/rsa: message too long for RSA key size". Replace the rune-based loop with a byte-slice loop using the correct OAEP-SHA256 maximum plaintext size: k − 2·hLen − 2 = 256 − 64 − 2 = 190 bytes for a 2048-bit key. Existing ciphertext is unaffected — DecryptLargeString is chunk-size agnostic. Also add a TODO comment on EncryptWithPublicRSAKey noting its SHA-512 / SHA-256 inconsistency with EncryptLargeString; no behavioural change there. Adds three regression tests covering box-drawing characters (U+2500, 3 B each), emoji (U+1F600, 4 B each), and a mid-rune byte-boundary split. Fixes: sc secrets add/hide failing with UTF-8 secret files. --- pkg/api/secrets/ciphers/encryption.go | 29 +++++++++++---- pkg/api/secrets/ciphers/encryption_test.go | 43 ++++++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/pkg/api/secrets/ciphers/encryption.go b/pkg/api/secrets/ciphers/encryption.go index 0424338d..116d5c75 100644 --- a/pkg/api/secrets/ciphers/encryption.go +++ b/pkg/api/secrets/ciphers/encryption.go @@ -82,7 +82,10 @@ func PublicKeyToBytes(pub *rsa.PublicKey) ([]byte, error) { return pubBytes, nil } -// EncryptWithPublicRSAKey encrypts data with public key +// EncryptWithPublicRSAKey encrypts data with public key. +// TODO: This uses SHA-512 (max plaintext 126 B for 2048-bit keys) while +// EncryptLargeString uses SHA-256 (max 190 B). Standardize these in a future PR; +// changing the hash here would break decryption of existing payloads. func EncryptWithPublicRSAKey(msg []byte, pub *rsa.PublicKey) ([]byte, error) { hash := sha512.New() ciphertext, err := rsa.EncryptOAEP(hash, rand.Reader, pub, msg, nil) @@ -147,14 +150,26 @@ func ParsePublicKey(s string) (crypto.PublicKey, error) { func EncryptLargeString(key crypto.PublicKey, s string) ([]string, error) { var res []string if rsaKey, ok := key.(*rsa.PublicKey); ok { - chunks := lo.ChunkString(s, rsaKey.Size()/2) - res = make([]string, len(chunks)) - for idx, chunk := range chunks { - encryptedData, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, []byte(chunk), nil) + // RSA-OAEP with SHA-256: max plaintext = k − 2·hLen − 2 bytes. + // For a 2048-bit key: 256 − 64 − 2 = 190 bytes. + // Chunking by rune count (old behaviour) is unsafe for multi-byte UTF-8: + // a 128-rune chunk can be up to 512 bytes, exceeding the 190-byte OAEP limit. + maxPlain := rsaKey.Size() - 2*sha256.Size - 2 + if maxPlain <= 0 { + return nil, errors.Errorf("RSA key too small (%d bits) for OAEP-SHA256", rsaKey.Size()*8) + } + data := []byte(s) + res = make([]string, 0, (len(data)+maxPlain-1)/maxPlain) + for i := 0; i < len(data); i += maxPlain { + end := i + maxPlain + if end > len(data) { + end = len(data) + } + enc, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, data[i:end], nil) if err != nil { - return nil, errors.Wrapf(err, "failed to encrypt secret") + return nil, errors.Wrap(err, "failed to encrypt secret") } - res[idx] = base64.StdEncoding.EncodeToString(encryptedData) + res = append(res, base64.StdEncoding.EncodeToString(enc)) } } else if ed25519Key, ok := key.(ed25519.PublicKey); ok { // For ed25519, use hybrid encryption with Curve25519 + ChaCha20-Poly1305 diff --git a/pkg/api/secrets/ciphers/encryption_test.go b/pkg/api/secrets/ciphers/encryption_test.go index af2d1772..9bd43d5f 100644 --- a/pkg/api/secrets/ciphers/encryption_test.go +++ b/pkg/api/secrets/ciphers/encryption_test.go @@ -102,6 +102,49 @@ func TestRSAEncryptionDecryption(t *testing.T) { Expect(err).To(BeNil()) Expect(string(decrypted)).To(Equal(largeData)) }) + + t.Run("RSA encrypt/decrypt UTF-8 multi-byte string (box-drawing)", func(t *testing.T) { + // U+2500 BOX DRAWINGS LIGHT HORIZONTAL = 3 bytes UTF-8. + // 120 runes × 3 bytes = 360 bytes. + // Old rune-based chunking (128 runes/chunk) produced one 360-byte chunk > 190 B → FAIL. + utf8Heavy := strings.Repeat("─", 120) + + encryptedChunks, err := EncryptLargeString(pubKey, utf8Heavy) + Expect(err).To(BeNil()) + Expect(encryptedChunks).NotTo(BeEmpty()) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(utf8Heavy)) + }) + + t.Run("RSA encrypt/decrypt emoji-dense string", func(t *testing.T) { + // U+1F600 GRINNING FACE = 4 bytes UTF-8. 100 runes = 400 bytes. + emojiHeavy := strings.Repeat("😀", 100) + + encryptedChunks, err := EncryptLargeString(pubKey, emojiHeavy) + Expect(err).To(BeNil()) + Expect(encryptedChunks).NotTo(BeEmpty()) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(emojiHeavy)) + }) + + t.Run("RSA chunk boundaries do not corrupt multi-byte sequences", func(t *testing.T) { + // 64 × 3 bytes (U+2500) = 192 bytes → exactly 2 chunks (190 + 2). + // Byte boundary at 190 falls mid-rune (U+2500 is bytes E2 94 80). + // Correct round-trip proves byte-boundary splitting and join are safe. + crossBoundary := strings.Repeat("─", 64) + + encryptedChunks, err := EncryptLargeString(pubKey, crossBoundary) + Expect(err).To(BeNil()) + Expect(len(encryptedChunks)).To(Equal(2)) + + decrypted, err := DecryptLargeString(privKey, encryptedChunks) + Expect(err).To(BeNil()) + Expect(string(decrypted)).To(Equal(crossBoundary)) + }) } func TestEd25519EncryptionDecryption(t *testing.T) { From a3202fe303f75159ae6187e9ae73f4ae82f173f0 Mon Sep 17 00:00:00 2001 From: "David Black (developer)" Date: Fri, 12 Jun 2026 17:08:26 +0000 Subject: [PATCH 5/7] forge: character developer (David Black) phase run idempotency: developer:run Co-authored-by: David Black (developer) --- .../agents/developer/handoff.json | 8 ++ .../agents/developer/handoff.md | 93 +++++++++++++++++++ .../agents/developer/summary.md | 1 + 3 files changed, 102 insertions(+) create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.json create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.md create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/summary.md diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.json b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.json new file mode 100644 index 00000000..b79c3637 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.json @@ -0,0 +1,8 @@ +{ + "bodyBytes": 4296, + "characterId": "developer", + "displayName": "David Black", + "phase": "run", + "roleHint": "developer", + "writtenAt": "2026-06-12T17:08:23.950728602Z" +} \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.md new file mode 100644 index 00000000..0236a240 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/handoff.md @@ -0,0 +1,93 @@ +Both files committed by David Black in this run. Here is the handoff. + +--- + +# Developer Handoff — Run `d97cc852-279d-4b65-affb-3d6f61fb8f82` +**Date:** 2026-06-12 +**From:** David Black (developer) +**Repo:** `simple-container-com/api` +**Branch:** `forge/workflow/99171cbe-7c27-4976-a999-931b834b5d62/run/d97cc852-279d-4b65-affb-3d6f61fb8f82` +**Commit:** `e120d5f3dc9d1b5a913deec3409b29fe97013a94` +**PR:** https://github.com/simple-container-com/api/pull/319 +**Workflow run:** https://app.simple-forge.com/agent-workflow-runs/d97cc852-279d-4b65-affb-3d6f61fb8f82 + +PULL_REQUEST: https://github.com/simple-container-com/api/pull/319 + +--- + +## What was done + +### Root cause + +`EncryptLargeString` in `pkg/api/secrets/ciphers/encryption.go` used `lo.ChunkString(s, rsaKey.Size()/2)` to split plaintext. `lo.ChunkString` counts **runes**. For a 2048-bit key `rsaKey.Size() = 256`, so chunk size = 128 runes. A 128-rune string of 3-byte UTF-8 characters (e.g. box-drawing `─`) encodes to 384 bytes — far above the RSA-OAEP-SHA256 maximum of `256 − 64 − 2 = 190 bytes`. Result: `crypto/rsa: message too long for RSA key size`. + +### Fix applied — `pkg/api/secrets/ciphers/encryption.go` + +Replaced the entire RSA branch of `EncryptLargeString`: + +```go +// BEFORE (buggy — rune-count chunking) +chunks := lo.ChunkString(s, rsaKey.Size()/2) +res = make([]string, len(chunks)) +for idx, chunk := range chunks { + encryptedData, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, []byte(chunk), nil) + ... + res[idx] = base64.StdEncoding.EncodeToString(encryptedData) +} + +// AFTER (correct — byte-slice chunking with exact OAEP limit) +maxPlain := rsaKey.Size() - 2*sha256.Size - 2 // 190 bytes for 2048-bit key +if maxPlain <= 0 { + return nil, errors.Errorf("RSA key too small (%d bits) for OAEP-SHA256", rsaKey.Size()*8) +} +data := []byte(s) +res = make([]string, 0, (len(data)+maxPlain-1)/maxPlain) +for i := 0; i < len(data); i += maxPlain { + end := i + maxPlain + if end > len(data) { end = len(data) } + enc, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, data[i:end], nil) + if err != nil { + return nil, errors.Wrap(err, "failed to encrypt secret") + } + res = append(res, base64.StdEncoding.EncodeToString(enc)) +} +``` + +Also added a `// TODO:` comment on `EncryptWithPublicRSAKey` documenting the SHA-512/SHA-256 inconsistency. No functional change to that function. + +**`lo` import retained** — `lo.Map` is still used in `DecryptLargeString`. + +**`DecryptLargeString` unchanged** — decryption is chunk-list driven and chunk-size agnostic; old (rune-chunked) ciphertext decrypts correctly. + +### Tests added — `pkg/api/secrets/ciphers/encryption_test.go` + +Three new sub-tests inside `TestRSAEncryptionDecryption`: + +| Test name | Payload | Why | +|-----------|---------|-----| +| `RSA encrypt/decrypt UTF-8 multi-byte string (box-drawing)` | 120 × `─` = 360 bytes | Reproduces the original bug (was failing) | +| `RSA encrypt/decrypt emoji-dense string` | 100 × `😀` = 400 bytes | 4-byte code points, higher density | +| `RSA chunk boundaries do not corrupt multi-byte sequences` | 64 × `─` = 192 bytes → 2 chunks (190+2) | Byte boundary falls mid-rune; proves join is safe | + +Note on chunk-count math: the architect's handoff specified 127 runes = 381 bytes → 2 chunks. That is wrong — `ceil(381/190) = 3`. The correct value is 64 runes = 192 bytes → `ceil(192/190) = 2`. Fixed before committing; test passes. + +--- + +## Acceptance criteria — all met + +| # | Criterion | Status | +|---|-----------|--------| +| AC-1 | `go test ./pkg/api/secrets/ciphers/...` passes green | ✅ `ok 2.977s` | +| AC-2 | New UTF-8 tests fail on old code | ✅ (360-byte chunk > 190-byte limit by math) | +| AC-3 | Existing `RSA encrypt/decrypt large string` still passes | ✅ | +| AC-4 | `lo.ChunkString` removed from encryption path | ✅ `grep ChunkString` → 0 hits | +| AC-5 | No new external dependencies | ✅ `go mod tidy` clean | + +--- + +## What was NOT changed + +- `DecryptLargeString` — byte-transparent; no change required +- `EncryptWithPublicRSAKey` / `DecryptWithPrivateRSAKey` — SHA-512 single-shot path; TODO comment added, no functional change +- ed25519 hybrid path — already correct; untouched +- Public API surface, CLI flags, or wire format \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/summary.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/summary.md new file mode 100644 index 00000000..437e8fc9 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/developer/summary.md @@ -0,0 +1 @@ +Both files committed by David Black in this run. Here is the handoff. \ No newline at end of file From 0aea86cbcf2ebfe6f9bf0209f829007b3273e76a Mon Sep 17 00:00:00 2001 From: "Maria Currie (qa)" Date: Fri, 12 Jun 2026 17:21:49 +0000 Subject: [PATCH 6/7] forge: character qa (Maria Currie) phase run idempotency: qa:run Co-authored-by: Maria Currie (qa) --- .../agents/qa/handoff.json | 8 + .../agents/qa/handoff.md | 166 ++++++++++++++++++ .../agents/qa/summary.md | 1 + 3 files changed, 175 insertions(+) create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.json create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.md create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/summary.md diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.json b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.json new file mode 100644 index 00000000..70a51a29 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.json @@ -0,0 +1,8 @@ +{ + "bodyBytes": 7214, + "characterId": "qa", + "displayName": "Maria Currie", + "phase": "run", + "roleHint": "qa", + "writtenAt": "2026-06-12T17:21:47.707860835Z" +} \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.md new file mode 100644 index 00000000..f1915a50 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/handoff.md @@ -0,0 +1,166 @@ +All CI checks are settled. 20/21 pass; only DCO fails — and that is a known infrastructure limitation of forge-agent commits (no `Signed-off-by` trailer). Every substantive check — build, tests, CodeQL, vulncheck, security scans, semgrep — is green. The code is correct. + +Now I'll write the QA handoff document. + +--- + +# QA Handoff — Run `d97cc852-279d-4b65-affb-3d6f61fb8f82` +**Date:** 2026-06-12 +**From:** Maria Currie (QA) +**PR:** https://github.com/simple-container-com/api/pull/319 +**Commit:** `e120d5f3dc9d1b5a913deec3409b29fe97013a94` +**Workflow run:** https://app.simple-forge.com/agent-workflow-runs/d97cc852-279d-4b65-affb-3d6f61fb8f82 + +PULL_REQUEST: https://github.com/simple-container-com/api/pull/319 + +--- + +**Verdict:** signoff + +--- + +## 1 — What I Verified + +I read both committed files directly from the branch and independently verified every acceptance criterion. I also waited for all CI checks to complete before issuing this verdict. + +--- + +## 2 — Code Review: `pkg/api/secrets/ciphers/encryption.go` + +### Fix correctness — PASS + +The `EncryptLargeString` RSA branch now uses: + +```go +maxPlain := rsaKey.Size() - 2*sha256.Size - 2 +``` + +Math checked: `256 − 2×32 − 2 = 190` bytes for a 2048-bit key. This is the exact RSA-OAEP-SHA256 maximum and matches the formula in the brief. + +The byte-slice loop is correct: + +```go +data := []byte(s) +res = make([]string, 0, (len(data)+maxPlain-1)/maxPlain) +for i := 0; i < len(data); i += maxPlain { + end := i + maxPlain + if end > len(data) { end = len(data) } + enc, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, rsaKey, data[i:end], nil) + ... +} +``` + +- Chunk size is bounded by `maxPlain = 190` → no chunk can exceed the OAEP limit. ✓ +- Pre-allocated slice with exact capacity. ✓ +- Key-too-small guard for degenerate cases. ✓ +- `errors.Errorf` / `errors.Wrap` from `github.com/pkg/errors` (already imported). ✓ + +### Import hygiene — PASS + +`lo.ChunkString` removed from encryption path; `lo` import retained because `lo.Map` is still used in `DecryptLargeString`. ✓ (`grep ChunkString` → 0 hits confirmed by reading the file.) + +### `DecryptLargeString` unchanged — PASS + +Decryption joins bytes via `strings.Join(lo.Map(decrChunks, ...))`. Because Go strings are byte sequences, this reassembles the original byte stream correctly even when a chunk boundary falls mid-rune. ✓ + +### TODO comment on `EncryptWithPublicRSAKey` — PASS + +Comment is present and accurate: + +> TODO: This uses SHA-512 (max plaintext 126 B for 2048-bit keys) while EncryptLargeString uses SHA-256 (max 190 B). Standardize these in a future PR; changing the hash here would break decryption of existing payloads. + +No functional change to that function. ✓ + +### ed25519 path — PASS + +Untouched. ✓ + +--- + +## 3 — Code Review: `pkg/api/secrets/ciphers/encryption_test.go` + +Three new sub-tests added inside `TestRSAEncryptionDecryption`. + +### Test 1: `RSA encrypt/decrypt UTF-8 multi-byte string (box-drawing)` +- Payload: `strings.Repeat("─", 120)` = 120 × 3 bytes = **360 bytes** +- Old code: all 120 runes in one chunk → 360 B > 190 B limit → would have failed. ✓ +- New code: ceil(360/190) = 2 chunks, both ≤ 190 B → succeeds. ✓ +- Round-trip `Expect(string(decrypted)).To(Equal(utf8Heavy))` — correct. ✓ + +### Test 2: `RSA encrypt/decrypt emoji-dense string` +- Payload: `strings.Repeat("😀", 100)` = 100 × 4 bytes = **400 bytes** +- Old code: one 400-byte chunk → OAEP fail. ✓ +- New code: ceil(400/190) = 3 chunks. ✓ + +### Test 3: `RSA chunk boundaries do not corrupt multi-byte sequences` +- Payload: `strings.Repeat("─", 64)` = 64 × 3 bytes = **192 bytes** +- Expected chunks: ceil(192/190) = **2** — assertion `Expect(len(encryptedChunks)).To(Equal(2))` is mathematically correct. ✓ +- Chunk 1: bytes[0:190] — ends at offset 1 within rune 64 (bytes: `...E2`), i.e., mid-rune. ✓ +- Chunk 2: bytes[190:192] — completes rune 64 (`94 80`). ✓ +- Round-trip reassembly confirms byte-transparent join works. ✓ + +**Note:** The developer correctly caught and fixed an error in the architect's spec. The architect specified 127 runes = 381 bytes and claimed 2 chunks; `ceil(381/190) = 3`, not 2. The committed test uses 64 runes = 192 bytes → exactly 2 chunks. This is the correct value and the assertion will not flake. + +### Existing test `RSA encrypt/decrypt large string` — PASS +- Payload: `testData` (57 ASCII bytes) × 20 = 1,140 bytes → ceil(1140/190) = 6 chunks, all ≤ 190 B. Backward compat intact. ✓ + +--- + +## 4 — Acceptance Criteria Verdict + +| # | Criterion | Result | Evidence | +|---|-----------|--------|---------| +| AC-1 | `go test ./pkg/api/secrets/ciphers/...` passes green | **PASS** | CI "Run tests": SUCCESS | +| AC-2 | New UTF-8 tests fail on old code | **PASS** | Math: 360 B / 400 B / 192 B all exceed old 128-rune=128-B-per-chunk limit | +| AC-3 | Existing `RSA encrypt/decrypt large string` still passes | **PASS** | CI "Run tests": SUCCESS; ASCII payload unaffected | +| AC-4 | `lo.ChunkString` removed from RSA encryption path | **PASS** | Read code: byte-slice loop used; no `ChunkString` call | +| AC-5 | No new external dependencies | **PASS** | `sha256.Size` is stdlib; CI build + mod tidy clean | + +All 5 ACs met. + +--- + +## 5 — CI Check Summary (final state) + +| Check | Result | +|-------|--------| +| Build Setup (clean, tools, schemas, lint, fmt) | SUCCESS | +| Run tests | SUCCESS | +| Build sc for linux/amd64 | SUCCESS | +| Build sc for darwin/arm64 | SUCCESS | +| Build sc for darwin/amd64 | SUCCESS | +| Build cloud-helpers | SUCCESS | +| Build github-actions | SUCCESS | +| Build github-actions-staging | SUCCESS | +| Finalize build in branch | SUCCESS | +| CodeQL | SUCCESS | +| govulncheck (reachability-aware) | SUCCESS | +| Fuzz HMAC cache parse path | SUCCESS | +| Semgrep | SUCCESS | +| security / Secret Scan (TruffleHog) | SUCCESS | +| security / SBOM + Dependency Scan | SUCCESS | +| security / Security Status | SUCCESS | +| **Developer Certificate of Origin** | **FAILURE** | + +The DCO failure is an infrastructure issue: forge-agent commits do not include a `Signed-off-by` trailer. This is not a code defect and is the same pattern as every other forge-agent commit in this branch. All substantive checks — build, tests, security, static analysis — are green. + +--- + +## 6 — Findings + +No blocking findings. One informational note for the record: + +1. **(Info) Architect spec error, caught by developer.** The cross-boundary test spec stated 127 runes = 381 bytes → 2 chunks; `ceil(381/190) = 3`, not 2. The developer caught this and used 64 runes (192 bytes → 2 chunks) correctly. The test assertion is deterministic and will not flake. + +2. **(Info) DCO failure is infrastructure-only.** All functional checks green. No action needed from the engineering team on the code itself. + +--- + +## 7 — Sign-off Scope + +This review covers: +- `pkg/api/secrets/ciphers/encryption.go` — fix and TODO comment +- `pkg/api/secrets/ciphers/encryption_test.go` — three new regression tests +- Full CI check suite (21 checks, 20 green, 1 infrastructure-only DCO failure) + +The fix is minimal, correct, backward-compatible, and fully covered by regression tests. Ready for merge. \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/summary.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/summary.md new file mode 100644 index 00000000..6c916e9b --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/qa/summary.md @@ -0,0 +1 @@ +All CI checks are settled. 20/21 pass; only DCO fails — and that is a known infrastructure limitation of forge-agent commits (no `Signed-off-by` trailer). Every substantive check — build, tests, CodeQL, vulncheck, security scans, semgrep — is green. The code is correct. \ No newline at end of file From 6fb75a3005bd0661863f38777168676b259880f8 Mon Sep 17 00:00:00 2001 From: "William Smith (devops)" Date: Fri, 12 Jun 2026 17:22:44 +0000 Subject: [PATCH 7/7] forge: character devops (William Smith) phase run idempotency: devops:run Co-authored-by: William Smith (devops) --- .../agents/devops/handoff.json | 8 ++ .../agents/devops/handoff.md | 113 ++++++++++++++++++ .../agents/devops/summary.md | 1 + 3 files changed, 122 insertions(+) create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.json create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.md create mode 100644 .workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/summary.md diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.json b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.json new file mode 100644 index 00000000..2f8aa6f6 --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.json @@ -0,0 +1,8 @@ +{ + "bodyBytes": 4944, + "characterId": "devops", + "displayName": "William Smith", + "phase": "run", + "roleHint": "devops", + "writtenAt": "2026-06-12T17:22:42.411355629Z" +} \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.md new file mode 100644 index 00000000..69137fed --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/handoff.md @@ -0,0 +1,113 @@ +Good. I've confirmed the fix is live on the branch and the PR is open with the correct state. The code in `encryption.go` is exactly what the fix requires. Now let me produce the DevOps handoff. + +--- + +# DevOps Handoff — Run `d97cc852-279d-4b65-affb-3d6f61fb8f82` +**Date:** 2026-06-12 +**From:** William Smith (DevOps) +**PR:** https://github.com/simple-container-com/api/pull/319 +**Commit (fix):** `e120d5f3dc9d1b5a913deec3409b29fe97013a94` +**Head SHA:** `0aea86cbcf2ebfe6f9bf0209f829007b3273e76a` +**Branch:** `forge/workflow/99171cbe-7c27-4976-a999-931b834b5d62/run/d97cc852-279d-4b65-affb-3d6f61fb8f82` + +PULL_REQUEST: https://github.com/simple-container-com/api/pull/319 + +--- + +## 1 — State Verified + +I fetched both the PR page and the raw source of `pkg/api/secrets/ciphers/encryption.go` from the branch directly. The fix is committed and matches spec exactly: + +- `EncryptLargeString` RSA branch: byte-slice loop, `maxPlain = rsaKey.Size() - 2*sha256.Size - 2` (190 bytes for 2048-bit) ✓ +- `lo.ChunkString` removed from RSA path ✓ +- `lo` import retained (`lo.Map` still used in `DecryptLargeString`) ✓ +- `// TODO:` comment on `EncryptWithPublicRSAKey` re SHA-512/SHA-256 inconsistency ✓ +- PR is **OPEN**, targeting `main` ✓ + +--- + +## 2 — CI Signal + +QA confirmed 20/21 checks pass. The single failure is **DCO (Developer Certificate of Origin)** — no `Signed-off-by` trailer on forge-agent commits. This is an infrastructure limitation of the forge workflow engine, not a code defect. + +| Check category | Result | +|----------------|--------| +| Build + lint + fmt | ✅ | +| Tests (`go test ./pkg/api/secrets/ciphers/...`) | ✅ | +| All 3 new UTF-8 regression tests | ✅ | +| CodeQL | ✅ | +| govulncheck | ✅ | +| Semgrep | ✅ | +| TruffleHog secret scan | ✅ | +| SBOM + dependency scan | ✅ | +| Build sc (linux/amd64, darwin/arm64, darwin/amd64) | ✅ | +| DCO | ❌ (infrastructure-only, no code fix needed) | + +--- + +## 3 — Operational Risk Assessment + +**Risk: None for this change.** Assessment: + +| Concern | Assessment | +|---------|-----------| +| Backward compat — existing encrypted secrets | **Safe.** `DecryptLargeString` is chunk-size agnostic. Old rune-chunked ciphertext decrypts correctly. | +| Re-encryption after merge | Not required. Existing secrets remain decryptable. Users may optionally force re-encrypt to produce tighter byte-aligned chunks. | +| Key size regression | Guarded: `if maxPlain <= 0` returns an error. No silent misencryption possible. | +| Import side-effects | `lo` import retained. `go mod tidy` clean. Zero new dependencies. | +| Wire format change | None. Encrypted payload format (base64-encoded RSA-OAEP ciphertext per chunk) is unchanged. | +| `DecryptLargeString` | Untouched. No risk. | +| ed25519 hybrid path | Untouched. No risk. | + +**DCO failure:** This is endemic to all forge-agent commits in this repo. It does not gate merge for maintainers with admin access. The substantive security, quality, and correctness checks are all green. + +--- + +## 4 — Merge Readiness + +``` +✅ Fix is correct (verified against source) +✅ Regression tests cover the exact failure modes +✅ All substantive CI checks green +✅ No new dependencies +✅ Backward compatible +✅ QA signed off +⚠️ DCO failure — infrastructure-only, maintainer can override +``` + +**Recommendation: Merge PR #319.** No pre-merge steps required. No post-merge operational actions required (no force re-encrypt, no config change, no deployment artifact change). + +--- + +## 5 — Post-Merge Notes + +- **No deployment action needed.** This is a library fix in `pkg/api/secrets/ciphers/`. It takes effect the next time `sc secrets add` / `sc secrets hide` is invoked by a user — no restart, no rollout, no config change required. +- **Future work (not blocking):** The `EncryptWithPublicRSAKey` SHA-512 inconsistency is documented with a `// TODO:` comment. Schedule a follow-up PR to standardize to SHA-256 and add hybrid encryption (RSA wraps AES-GCM key). Neither is urgent. +- **DCO infrastructure:** If the team wants forge-agent commits to pass DCO in future, configure the bot identity to include `Signed-off-by` trailers, or add a DCO exemption for the forge-agent app in the repo's DCO config. + +--- + +## 6 — Handoff Summary + +| Item | Status | +|------|--------| +| Fix in repo | ✅ committed at `e120d5f` | +| Tests added | ✅ 3 UTF-8 regression tests | +| CI green (substantive) | ✅ 20/21 (DCO infra-only) | +| QA sign-off | ✅ | +| Merge blocker | None (DCO requires maintainer override) | +| Post-merge action | None required | + +```json +{ + "run_id": "d97cc852-279d-4b65-affb-3d6f61fb8f82", + "from_character": "devops", + "pr": "https://github.com/simple-container-com/api/pull/319", + "fix_commit": "e120d5f3dc9d1b5a913deec3409b29fe97013a94", + "merge_ready": true, + "blockers": [], + "dco_failure": "infrastructure-only, maintainer override acceptable", + "post_merge_actions": [], + "chainContinue": false +} +``` \ No newline at end of file diff --git a/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/summary.md b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/summary.md new file mode 100644 index 00000000..d52e296e --- /dev/null +++ b/.workflow/d97cc852-279d-4b65-affb-3d6f61fb8f82/agents/devops/summary.md @@ -0,0 +1 @@ +Good. I've confirmed the fix is live on the branch and the PR is open with the correct state. The code in `encryption.go` is exactly what the fix requires. Now let me produce the DevOps handoff. \ No newline at end of file