This is the fourth in a series of posts wherein I attempt to recount the history of Goread2 as it approaches a state in which I might actually try to share it more broadly.

There is a gap between a working app and a trustworthy one, and bridging that gap requires a different kind of work than adding features. Security and stability work tends to be subtractive—you read code you already wrote with fresh, suspicious eyes, and ask: what could go wrong here? What happens when this fails? Who could abuse this, and how? And you find yourself slapping your own forehead a lot, and wishing you’d been more mindful with your AI prompting.

In mid-November, about three and a half months after the first deployment, there was much forehead slappage occurring. The commit that kicked it off was titled Add code review findings for internal/services package, and it dropped nineteen issues into the issue tracker in a single shot: two P0 criticals, seven P1 highs, seven P2 mediums, and three P3 lows, all generated from a careful read of nine files in the service layer. The issues ranged from race conditions to goroutine leaks to missing timeouts to an OAuth vulnerability that had been sitting there quietly for months. What followed was two days of systematic fixes, a concentrated hardening sprint that touched almost every layer of the application.

But to understand November, you have to go back to October, when a few of the more alarming issues had already been quietly lurking.

A Brief Retrospective on October’s Security Findings

On October 16, I noticed this block in feed_service.go:

if os.Getenv("GAE_ENV") == "standard" {
    transport.TLSClientConfig = &tls.Config{
        InsecureSkipVerify: true,
    }
}

This code, running in production on every feed fetch, disabled TLS certificate verification. Completely. For all HTTPS connections. On every request to every RSS feed on the internet.

The commit that removed said code explained: “This was based on the incorrect assumption that App Engine lacks CA certificates. In reality, App Engine Standard environment includes standard CA certificates and the default http.Client works correctly.” The “fix” that had been in place was one of those well-intentioned patches that solves an imaginary problem while creating a real one: in this case, a man-in-the-middle vulnerability on every feed fetch. The commit message helpfully labeled it CRITICAL.

Next, on October 17, we have Fix CRITICAL SSRF vulnerability in feed URL handling. Server-Side Request Forgery is what you get when a user can supply a URL and the server will cheerfully fetch it without checking where it points. The attack surface¹ is obvious: a user submits http://169.254.169.254/latest/meta-data/ as a feed URL, and the server fetches Google Cloud’s instance metadata endpoint, potentially returning API keys and service account tokens. Or they submit http://192.168.1.1 and start probing the internal network. Before the fix, GoRead2 would have obliged. The fix was to add the URLValidator that blocks all RFC 1918 ranges, loopback addresses, link-local addresses, and validates DNS resolution to catch rebinding attacks. This became one of the more robust pieces of security infrastructure in the codebase.

The November Sprint

Back to November 15, and the code review that resulted in nineteen new issues. Let’s group them by category.

The first category was memory leaks. The caching layer that had been carefully built in October to reduce Datastore reads had a quiet flaw in both of its implementations: when cache entries expired, they stopped being returned but were never actually deleted. UnreadCache stored per-user unread counts, so in theory, for a long-running instance with many users, every user who had ever triggered a cache entry would have their data sitting in memory forever, long after it expired. FeedListCache had the same issue. These were fixed by adding cleanup goroutines that ran every five minutes and deleted stale entries.

The second category was missing timeouts. All 50+ Datastore operations in datastore.go were using context.Background(), which carries no deadline. A Datastore operation that hangs has no way to time out; instead, the goroutine sits there waiting and holding resources indefinitely. The fix standardized all operations on a 30-second datastoreTimeout constant via a newDatastoreContext() helper, applied consistently everywhere. This is the kind of tedious change whose value shows up only in the rare production scenario where a database call goes wrong in a slow way rather than a fast way.

The third category was concurrency bugs. The per-request caching layer that deduplicated repeated calls within a single HTTP request had a race condition: multiple goroutines could be writing to it simultaneously. Fixed with a mutex. AddFeedForUser had a context timeout for the feed-fetch operation, but if that timeout fired, a goroutine was leaked: the deferred cleanup never ran because the function had already returned. Fixed by restructuring the cleanup. The unchecked type assertion in GetCachedUserFeeds would have panicked and crashed the server if the context ever held a value of the wrong type. Fixed with the comma-ok idiom.

The fourth category was authentication hardening. The OAuth state parameter, the random token generated during the login flow to prevent CSRF attacks, was being validated but not consumed. A valid state could be used multiple times within its ten-minute TTL window. This is a replay attack vector: an attacker who intercepts a valid OAuth callback URL can use it to authenticate as someone else, as long as they act within the window. The fix made states one-time-use: ValidateAndConsumeOAuthState() validates the state and deletes it in the same operation, so a replayed callback gets rejected.

The fifth category was simpler: the SecretManager client was being instantiated fresh on every call to GetSecret(). This was happening at startup and on every request that needed credentials. Converting it to a singleton substantially reduced connection overhead.

The November hardening sprint closed the highest-priority items from the code review within about 48 hours. What strikes me about this phase is how many of the vulnerabilities and bugs were architectural rather than incidental. TLS verification was disabled because the original code made a wrong assumption about the environment. Memory leaked because the cache design separated the “is this expired?” check from the “clean this up” action, which is a design mistake. The Datastore operations had no timeouts because nobody thought to add them when writing the initial implementation — there was no specification saying “all database calls must have a deadline,” so they didn’t. These aren’t the kinds of bugs you find by testing happy paths. You find them by reading the code with the specific goal of finding them.

There is a broader lesson buried in this chapter, and it is worth being direct about it. Most of the vulnerabilities and bugs described here were not hard to fix; rather, they were hard to notice, because nobody had specifically looked for them. That is a pattern that tends to emerge from AI-assisted development done at speed. When you are prompting an AI for functionality,you get code that does the thing. What you don’t automatically get is code that has been stress-tested against an adversarial mental model: what if this URL points somewhere it shouldn’t? What happens to these map entries in six months? Can this token be replayed? Prompting for features and prompting for security are different modes of thinking, and it is easy to stay in the first mode for too long because it is faster and the results are immediately visible. The TLS bypass, the SSRF exposure, and the leaking caches were all the result of code that correctly solved the stated problem while nobody had yet asked the harder questions. The November audit was valuable not because it was technically sophisticated, but because it was the first time the codebase was inspected with the explicit goal of finding what could go wrong. Treating security review as a first-class part of the development loop rather than a catch-up exercise from the outset would have been worth it. We have tried to be more deliberate about that since.

Next: we get serious about process.

1. I despise the phrase “attack surface” and other similar scary-sounding infosec jargon. So when you see stuff like this in my writing, you know I had an AI helper. ↩